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

Servo isn't exposing custom http response statuses to XHR #24924

Open
pshaughn opened this issue Nov 29, 2019 · 2 comments
Open

Servo isn't exposing custom http response statuses to XHR #24924

pshaughn opened this issue Nov 29, 2019 · 2 comments

Comments

@pshaughn
Copy link
Member

@pshaughn pshaughn commented Nov 29, 2019

WPT xhr/status-async, xhr/status-basic and xhr/status-error expect to be able to see custom http statuses like "204 UNICORNSWIN" and "699 WAY OUTTA RANGE". Servo is instead showing out-of-range status numbers as 0, and is returning the standardized string meanings of in-range status numbers instead of the custom ones the server sent. I'm not seeing the list of standard status names within Servo's own source tree, so this must be involving another crate, maybe http.

In some cases in status-basic it looks like it's even interpreting the custom responses as a network error!

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Nov 29, 2019

send-redirect-no-location and send-redirect-to-cors also fail like this.
edit: so do /xhr/send-contional and /xhr/send-conditional-cors
So does test2 of xhr/send-data-unexpected-tostring

@Darkspirit
Copy link
Contributor

@Darkspirit Darkspirit commented Dec 18, 2019

RUST_LOG=net ./mach test-wpt tests/wpt/web-platform-tests/xhr/status-basic.htm

statusRequest("CHICKEN", code, text, content, type)

HttpMethod::from_str(m)

It seems the CHICKEN request method is correctly set.


status(401, "OH HELLO", "Not today.", "")

  • Status reason is read here:

    response.status = Some((
    res.status(),
    res.status().canonical_reason().unwrap_or("").into(),
    ));
    debug!("got {:?} response for {:?}", res.status(), request.url());
    response.raw_status = Some((
    res.status().as_u16(),
    res.status().canonical_reason().unwrap_or("").into(),
    ));

    hyperium/hyper#135 added status_raw for this XHR use case. But as we can see above, Servo (now) uses canonical_reason() for both.

    That's because hyperium/hyper@3cd48b4#diff-80398c5faae3c069e4e6aa2ed11b28c0 and
    024b40b#diff-aa469beb5619907dbccd88364264b9b8L1088 removed the raw_status feature.

  • HTTP/1.1 401 is Unauthorized by definition: https://tools.ietf.org/html/rfc7235#section-3.1.
    But RFC 2616 also says:

    The reason phrases listed here are only recommendations -- they MAY be replaced by local equivalents without affecting the protocol

    hyper's http crate reminds us:

    Bear in mind also that in HTTP/2.0 and HTTP/3.0 the reason phrase is abolished from transmission, and so this canonical reason phrase really is the only reason phrase you’ll find.

    Therefore canonical_reason() is only derived from status code (number).

    But the fetch spec possibly wants us to remove it when using HTTP/2: https://fetch.spec.whatwg.org/#concept-response-status-message says

    Responses over an HTTP/2 connection will always have the empty byte sequence as status message as HTTP/2 does not support them.

  • Either HTTP/1.1 raw_status needs to be added back to hyper or information should be gathered whether this could be wontfixed or the spec be updated.

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.