Skip to content

Allow sending cookies on XMLHttpRequest.send() #44

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

carlosefr
Copy link

@carlosefr carlosefr commented Apr 11, 2025

XMLHttpRequest.send() does not send any cookies that the browser may have for the requested URL unless withCredentials = True. This PR enables that flag by default allows setting that flag as an option.

This allows doing requests to URLs that depend on cookie-based authentication (e.g. where the user authenticates in one browser tab/window, and the pyodide application is running in another tab/window).

I tested this by overriding the pyodide_http._core.send() function in a Marimo WASM notebook, and then doing a requests.get() against an URL protected by Cloudflare Access (which uses cookies as one of the possible authentication methods). This is the code I used:

import js
import pyodide_http
import pyodide_http._core as ph
from pyodide.ffi import to_js
from email.parser import Parser
from pyodide_http._streaming import send_streaming_request

def my_send(request: ph.Request, stream: bool = False) -> ph.Response:
    js.eval("console.log('send() override')")

    if request.params:
        from js import URLSearchParams

        params = URLSearchParams.new()
        for k, v in request.params.items():
            params.append(k, v)
        request.url += "?" + params.toString()

    from js import XMLHttpRequest

    try:
        from js import importScripts  # noqa

        _IN_WORKER = True
    except ImportError:
        _IN_WORKER = False
    # support for streaming workers (in worker )
    if stream:
        if not _IN_WORKER:
            stream = False
            ph.show_streaming_warning()
        else:
            result = send_streaming_request(request)
            if result == False:  # noqa
                stream = False
            else:
                return result

    xhr = XMLHttpRequest.new()
    # set timeout only if pyodide is in a worker, because
    # there is a warning not to set timeout on synchronous main thread
    # XMLHttpRequest https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout
    if _IN_WORKER and request.timeout != 0:
        xhr.timeout = int(request.timeout * 1000)

    if _IN_WORKER:
        xhr.responseType = "arraybuffer"
    else:
        xhr.overrideMimeType("text/plain; charset=ISO-8859-15")

    xhr.open(request.method, request.url, False)
    for name, value in request.headers.items():
        if name.lower() not in ph.HEADERS_TO_IGNORE:
            xhr.setRequestHeader(name, value)

    xhr.withCredentials = True
    xhr.send(to_js(request.body))

    headers = dict(Parser().parsestr(xhr.getAllResponseHeaders()))

    if _IN_WORKER:
        body = xhr.response.to_py().tobytes()
    else:
        body = xhr.response.encode("ISO-8859-15")

    return ph.Response(status_code=xhr.status, headers=headers, body=body)

ph.send = my_send
pyodide_http.patch_all()

@koenvo
Copy link
Owner

koenvo commented May 8, 2025

Thanks for the PR! I think this would be better as an opt-in feature—enabling withCredentials by default could cause unexpected issues (e.g., with CORS or CSRF). Could we make it configurable via patch_all(with_credentials=True) or something similar?

@carlosefr
Copy link
Author

To be able to test these changes, I had to do some fixes to the test suite. I wasn't able to run it as is on macOS 15.4.1 (Apple Silicon) with Chrome 136.0.7103.93.

Still, some issues remain:

  1. Some tests from test_streaming.py still fail with a timeout. This seems unrelated to the credentials changes that are the object of this PR. It seems related to running in an isolated environment (see below).
  2. When using with_credentials=True, the tests in test_non_streaming.py fail with an exception. It fails even if I just do xhr.withCredentials = True like in the first iteration of this PR, which is strange given that my manual tests inside Marimo (see above) demonstrate that this should work.

@koenvo Any thoughts?


test_non_streaming.py::test_install_package[chrome] PASSED                                                                                                                     [  6%]
test_non_streaming.py::test_requests_get[chrome] PASSED                                                                                                                        [ 12%]
test_non_streaming.py::test_urllib_get[chrome] PASSED                                                                                                                          [ 18%]
test_non_streaming.py::test_requests_404[chrome] PASSED                                                                                                                        [ 25%]
test_streaming.py::test_requests_stream_worker[isolated-chrome] FAILED                                                                                                         [ 31%]
test_streaming.py::test_requests_404[isolated-chrome] PASSED                                                                                                                   [ 37%]
test_streaming.py::test_install_package_isolated[isolated-chrome] PASSED                                                                                                       [ 43%]
test_streaming.py::test_requests_stream_main_thread[isolated-chrome] PASSED                                                                                                    [ 50%]
test_streaming.py::test_response_headers[isolated-chrome] PASSED                                                                                                               [ 56%]
test_streaming.py::test_requests_parallel_stream_workers[isolated-chrome] FAILED                                                                                               [ 62%]
test_streaming.py::test_requests_stream_worker[non-isolated-chrome] PASSED                                                                                                     [ 68%]
test_streaming.py::test_requests_404[non-isolated-chrome] PASSED                                                                                                               [ 75%]
test_streaming.py::test_install_package_isolated[non-isolated-chrome] PASSED                                                                                                   [ 81%]
test_streaming.py::test_requests_stream_main_thread[non-isolated-chrome] PASSED                                                                                                [ 87%]
test_streaming.py::test_response_headers[non-isolated-chrome] PASSED                                                                                                           [ 93%]
test_streaming.py::test_requests_parallel_stream_workers[non-isolated-chrome] PASSED            

@carlosefr
Copy link
Author

carlosefr commented May 9, 2025

I was able to "fix" the failing tests from test_streaming.py by downgrading to pyodide 0.25.1. Those tests start failing with 0.26.x. So definitely not related to the with_credentials changes.

@carlosefr
Copy link
Author

Since setting with_credentials on patch meant choosing between sending implicity cookies or supporting Access-Control-Allow-Origin: * URLs, I transformed it into an option that can be set at any time or temporarily with a context manager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants