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

throwing all properties from url.parse into the request options is problematic #260

Open
eli-darkly opened this issue Mar 15, 2022 · 0 comments

Comments

@eli-darkly
Copy link

Currently, connect() starts by setting options to the result of parse(url), then it adds some more properties, and then it passes the object to http.request or https.request. However, while the options type for those functions uses many of the same properties as the return type of url.parse, it does not use all of them; in other words, there are some properties of a parsed URL that are not valid as request properties, such as hash.

Normally Node ignores any unrecognized properties, so why does this matter? It matters because middleware code may intercept functions like http.request and, in order to correctly handle the variant calling conventions of those functions, it will need to disambiguate between 1. this parameter is a URL string, 2. this parameter is a URL object, 3. this parameter is a request options object. Number 1 is easy, but distinguishing between 2 and 3 is harder since they have many properties in common. And this isn't just a theoretical concern— at least one middleware product is known to break for exactly this reason.

The solution would be to copy over the individual properties of the parsed URL that really are valid as request options, rather than taking them all and extending that object.

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

1 participant