Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server does not implement CORS correctly for credentialed requests #4511

Closed
4 tasks done
Azeirah opened this issue Dec 17, 2023 · 3 comments
Closed
4 tasks done

server does not implement CORS correctly for credentialed requests #4511

Azeirah opened this issue Dec 17, 2023 · 3 comments

Comments

@Azeirah
Copy link
Contributor

Azeirah commented Dec 17, 2023

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • I am running the latest code. Development is very rapid so there are no tagged versions as of now.
  • I carefully followed the README.md.
  • I searched using keywords relevant to my issue to make sure that I am creating a new issue that is not already open (or closed).
  • I reviewed the Discussions, and have a new bug or useful enhancement to share.

Expected Behavior

I expect to be able to send either credentialed or uncredentialed responses without a difference in behavior in the server.

read here on MDN about "credentials" in the heading "Functional overview"

Current Behavior

When sending a credentialed request , the llama.cpp server does not respond with the correct CORS preflight response.

Environment and Context

You can use this super simple llama client to test out the behavior. Write the url in the input field, for instance I have http://localhost:8083/v1/chat/completions) and press the "send with auth" button to see that it does not work. Open browser console to inspect that the OPTIONS flight request fails.

The almost identical request that does not use a credentialed request works as expected.

<!DOCTYPE html>
<html>
<head>
	<meta charset="utf-8">
	<meta name="viewport" content="width=device-width, initial-scale=1">
	<title>llama.cpp server CORS</title>
</head>
<body>
	<button class="without">Send without auth</button>
	<button class="with">Send with auth</button>

	<input type="text" class="url" placeholder="localhost:8080/v1/completion">

	<pre>
		<code class="out">
			
		</code>
	</pre>

	<script type="text/javascript">
		const $out = document.querySelector(".out");

		const $btn_without = document.querySelector(".without");
		const $btn_with = document.querySelector(".with");
		const $url = document.querySelector(".url");

		$btn_with.addEventListener("click", async function () {
			const res = await fetch($url.value, {
				method: "POST",
				headers: {
					Authorization: "no key",
					"Content-Type": "application/json",
				},
				body: JSON.stringify({
					messages: [{role: "user", content: "What is the color of the sky?"}]
				})
			});
			$out.innerText = JSON.stringify(JSON.parse(await res.text()), null, 4);
		});

		$btn_without.addEventListener("click", async function () {
			const res = await fetch($url.value, {
				method: "POST",
				headers: {
					"Content-Type": "application/json",
				},
				body: JSON.stringify({
					messages: [{role: "user", content: "What is the color of the sky?"}]
				})
			});
			$out.innerText = JSON.stringify(JSON.parse(await res.text()), null, 4);
		});
	</script>
</body>
</html>
@Azeirah
Copy link
Contributor Author

Azeirah commented Dec 17, 2023

This issue is relevant for browser-based clients that consume the server provided by llama.cpp. I've personally encountered it within an Obsidian plugin, but it can occur in any browser-based environment such as vscode, electron-based chat clients and any website.

My older post has some more details about how to implement this behavior correctly: #4198 (comment)

In short, for the preflight request

  • Response header Access-Control-Allow-Origin should be dynamically set to request.Origin, not *. That is the origin that the request came from, this is different for each application/computer/server etc, httplib.hpp stores this in req.origin iirc.
  • Response header Access-Control-Allow-Credentials should be set explicitly to true

I'm not 100% sure if any other headers have to be set to anything, I'd have to experiment a bit.

@phymbert
Copy link
Collaborator

@Azeirah AFAIK it's closed no ?

@Azeirah
Copy link
Contributor Author

Azeirah commented Feb 24, 2024

@Azeirah AFAIK it's closed no ?

Oh yes, I never closed the issue. Only the PR. Good point.

@Azeirah Azeirah closed this as completed Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants