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

[wasm] HttpResponseMessage.Headers is empty #53668

Closed
pavelsavara opened this issue Jun 3, 2021 · 5 comments · Fixed by #52923
Closed

[wasm] HttpResponseMessage.Headers is empty #53668

pavelsavara opened this issue Jun 3, 2021 · 5 comments · Fixed by #52923
Labels
arch-wasm WebAssembly architecture area-System.Net.Http
Milestone

Comments

@pavelsavara
Copy link
Member

pavelsavara commented Jun 3, 2021

Failing unit test System.Net.Http.Functional.Tests.HttpClientHandler_RemoteServerTest.SendAsync_SendRequestUsingMethodToEchoServerWithContent_Success for POST Echo.ashx

Caused by lack of CORS on server side, fix in progress.

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Net.Http labels Jun 3, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jun 3, 2021
@ghost
Copy link

ghost commented Jun 3, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Failing unit test System.Net.Http.Functional.Tests.HttpClientHandler_RemoteServerTest.SendAsync_SendRequestUsingMethodToEchoServerWithContent_Success for POST Echo.ashx

Author: pavelsavara
Assignees: -
Labels:

arch-wasm, area-System.Net.Http

Milestone: -

@pavelsavara
Copy link
Member Author

pavelsavara commented Jun 3, 2021

This is most likely caused by lack of Access-Control-Expose-Headers: *
See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers

Way to do this is add .WithExposedHeaders() to server startup.
See dotnet/xharness#624

@mdh1418
Copy link
Member

mdh1418 commented Jun 3, 2021

What was the failure log? Do we have a specific determined set of header names that we can set Access-Control-Expose-Headers?
I noticed that * will only work for requests without credentials, would this negatively impact the requests with credentials?

@pavelsavara
Copy link
Member Author

The list of specific header is dependent on the unit tests and the middleware. There could be many.
We could expose the list via CLI parameter, but this is localhost only, so security is not interesting here.

Credentials are not currently in scope for WASM as far as I could tell.

It may be more interesting for mobile unit tests which have different underlying http component.
Perhaps it doesn't send the Origin header and doesn't trigger CORS at all, as opposed to browser.

To make credentials work, you have to state host name explicitly and avoid AllowAnyOrigin AFAIK.
I would prefer to solve it in mobile related PR if necessary, not in this PR.

@pavelsavara
Copy link
Member Author

The specific log is not interesting, it just throws exception when looking for response header.
For example here https://github.com/dotnet/runtime/pull/52923/files#diff-46af1cf2c6e9ba436654ffec5d624c82dbb101aaf4ddde3e7154025370a8f668R78

@lewing lewing added this to the 6.0.0 milestone Jun 3, 2021
@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Jun 3, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 8, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net.Http
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants