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

Automatically replying to OPTIONS requests is potentially problematic #4551

Closed
MarceloZabini opened this issue Aug 30, 2022 · 10 comments · Fixed by #4559
Closed

Automatically replying to OPTIONS requests is potentially problematic #4551

MarceloZabini opened this issue Aug 30, 2022 · 10 comments · Fixed by #4559

Comments

@MarceloZabini
Copy link

MarceloZabini commented Aug 30, 2022

Bug report

OPTIONS requests are automatically handled by webpack-dev-server since #4180 was implemented by #4185, bypassing the application's backend.

Actual Behavior

webpack-dev-server automatically replies to OPTIONS requests regardless of how the backend application would handle it.

Expected Behavior

Much like any other kind of requests, like GET and POST, OPTIONS requests should by default be forwarded to the application. Applications can reply with a failure depending on the request, and can include a variety of response headers which can be important for the application.

How Do We Reproduce?

Use webpack-dev-server 4.10.0, which contains #4185, and any application that handles OPTIONS requests in the backend to see that the backend application isn't even contacted.

Please paste the results of npx webpack-cli info here, and mention other relevant information

System:
OS: Linux 5.14 Fedora 33 (Workstation Edition) 33 (Workstation Edition)
CPU: (8) x64 Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz
Memory: 7.60 GB / 15.38 GB
Binaries:
Node: 14.19.3 - /nix/store/cnma41dzr06jp8j6vigfp4lmy1dqfg3x-nodejs-14.19.3/bin/node
npm: 6.14.17 - /nix/store/cnma41dzr06jp8j6vigfp4lmy1dqfg3x-nodejs-14.19.3/bin/npm

@alexander-akait
Copy link
Member

alexander-akait commented Sep 2, 2022

@MarceloZabini What about if we implement an option for this? So you can disable it when you need

@MarceloZabini
Copy link
Author

MarceloZabini commented Sep 5, 2022

There is perhaps a discussion of what users expect the default to be (personally I'd expect every request to be proxied), but having the option available and documented would certainly be good enough - at least for me.

@alan-agius4
Copy link
Contributor

alan-agius4 commented Sep 5, 2022

This has also been reported in angular/angular-cli#23857

I wonder if it's just a matter of registering the options-middleware as the last middleware, which will act as a fallback if none of the other middlewares handles the OPTIONS.

@alan-agius4
Copy link
Contributor

alan-agius4 commented Sep 5, 2022

Are you able to test if by updating node_modules/webpack-dev-server/lib/Server.js to https://gist.github.com/alan-agius4/156a7c36c2927986ef4e1dc6ffcdc97a the issue still persists?

Edit: Based on the reproduction in angular/angular-cli#23859 it does seem like it's a matter of registration order.

@sek1973
Copy link

sek1973 commented Sep 5, 2022

We also have problems with OPTIONS requests handled this way.
I reported it to Angular team, as webpack-dev-server@4.10.0 is internally used by @angular-devkit/build-angular@14.2.1. Please find details here https://github.com/angular/angular-cli/issues/23857.
I also described a workaround for OPTIONS requests to be handled properly.

@alan-agius4
Copy link
Contributor

alan-agius4 commented Sep 5, 2022

Another user confirmed my suspicions that it’s a matter of ordering.

See: angular/angular-cli#23857 (comment)

@snitin315
Copy link
Member

snitin315 commented Sep 5, 2022

Feel free to send a PR.

@alexander-akait
Copy link
Member

alexander-akait commented Sep 5, 2022

@alan-agius4 hm, do you have a valid order (confirmed that we don't have the issue again)?

@alan-agius4
Copy link
Contributor

alan-agius4 commented Sep 6, 2022

@alexander-akait, yeah it need to be registered as the last middleware as per the gist (https://gist.github.com/alan-agius4/156a7c36c2927986ef4e1dc6ffcdc97a) that the above users used to verify this.

alan-agius4 added a commit to alan-agius4/webpack-dev-server that referenced this issue Sep 6, 2022
Prior to this change the internal options middleware always responsed to such requests. This is because the middleware was registered too early and was not being used as a fallback. With this change we register this middleware as the last middleware to ensure that this is only used as a fallback when OPTIONS requests are not handled.

Closes webpack#4551
rishabh3112 pushed a commit that referenced this issue Sep 7, 2022
Prior to this change the internal options middleware always responsed to such requests. This is because the middleware was registered too early and was not being used as a fallback. With this change we register this middleware as the last middleware to ensure that this is only used as a fallback when OPTIONS requests are not handled.

Closes #4551
@alexander-akait
Copy link
Member

alexander-akait commented Sep 7, 2022

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 a pull request may close this issue.

5 participants