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

support raw UTF8 paths #146

Open
lovasoa opened this issue Aug 14, 2023 · 10 comments
Open

support raw UTF8 paths #146

lovasoa opened this issue Aug 14, 2023 · 10 comments

Comments

@lovasoa
Copy link

lovasoa commented Aug 14, 2023

Hello !
I discovered recently that many user agents send raw UTF8 bytes in HTTP paths, and proxies forward them without issues. See actix/actix-web#3102

However, it looks like this library fails to parse such HTTP requests, preventing web servers from handling them.

Would it be possible to handle such requests in this library ?

@seanmonstar
Copy link
Owner

I feel like the bytes that httparse initially allowed were from an RFC, but I no longer remember which one it was (7230? URIs? WHATWG Fetch?)

@lovasoa
Copy link
Author

lovasoa commented Aug 30, 2023

But if actual user agents, including safari, do send requests with utf8 paths, shouldn't we support them in server-side libraries, even if there is an RFC saying they are invalid ?

@seanmonstar
Copy link
Owner

The general answer to that question is not always easy. There's sometimes good reasons to prevent them.

In this case, it could be fine to make the parser more relaxed. I was trying to remember what said to make it strict originally, so I could read if they included any comments as to why.

@JakeChampion
Copy link

JakeChampion commented Oct 27, 2023

I believe this is coming from rfc3986 section 3.3 and the definition for a path segment not allowing non-us-ascii characters

In particular it's the pchar definition, where characters outside the allowed set need to be percent encoded.

   path          = path-abempty    ; begins with "/" or is empty
                 / path-absolute   ; begins with "/" but not "//"
                 / path-noscheme   ; begins with a non-colon segment
                 / path-rootless   ; begins with a segment
                 / path-empty      ; zero characters

   path-abempty  = *( "/" segment )
   path-absolute = "/" [ segment-nz *( "/" segment ) ]
   path-noscheme = segment-nz-nc *( "/" segment )
   path-rootless = segment-nz *( "/" segment )
   path-empty    = 0<pchar>

   segment       = *pchar
   segment-nz    = 1*pchar
   segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" )
                 ; non-zero-length segment without any colon ":"

   pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"


   pct-encoded   = "%" HEXDIG HEXDIG

   unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
   sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
                 / "*" / "+" / "," / ";" / "="

The whole abnf can be view at either https://abnf-uri.edgecompute.app/ or https://www.rfc-editor.org/rfc/rfc3986.html#appendix-A

right now applications which use httparse can be sure that it returns a valid uri according to rfc3986

perhaps a feature flag could be added to allow utf8 (rfc3986-non-compliant) path support, so that applications can choose whether they want httparse to be strict or relaxed?

@lovasoa
Copy link
Author

lovasoa commented Oct 27, 2023

But if there is a flag, is it realistic to think people will care enough to activate it ? I only learned about this after getting bitten by it, and httparse is probably a sub-sub-dependency for most people who just want to handle http requests made by web browsers. I think it would be great if httparse were able to parse web requests made by Safari by default, and if the update just silently fixed this issue that is currently present by transitivity almost everywhere in the rust server framework ecosystem.

@banchen19
Copy link

use Websocket for ws://127.0.0.1:2000/api/pe/ws?server_name=阿
this use chinese path
ERROR [actix_http::h1::dispatcher] stream error: request parse error: invalid Header provided

@seanmonstar
Copy link
Owner

I'd prefer to check with a few places that proxy serious Internet traffic if this is something they see, or have reasons to not want it.

cc @Noah-Kennedy @nox @acfoltzer

@JakeChampion
Copy link

I'd prefer to check with a few places that proxy serious Internet traffic if this is something they see, or have reasons to not want it.

cc @Noah-Kennedy @nox @acfoltzer

When I worked at Fastly we did see this from some customers traffic, which is why I ended up reading the RFC and commenting above on this issue 👍

@nox
Copy link
Contributor

nox commented Sep 13, 2024

@seanmonstar We are already running a httparse fork for some products to support raw UTF-8 paths.

@joelwurtz
Copy link

We are also observing this for some of customers where they hit this error because of utf8 char in path, we also use a fork to support this

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

No branches or pull requests

6 participants