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.

Sign up for GitHub

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

Jump to bottom

The parseHeaders in xhrHttpsClient.ts and xhrHttpClient.ts is not strong enough #11581

Closed
1 of 6 tasks
ewfian opened this issue Oct 1, 2020 · 6 comments
Closed
1 of 6 tasks

The parseHeaders in xhrHttpsClient.ts and xhrHttpClient.ts is not strong enough #11581

ewfian opened this issue Oct 1, 2020 · 6 comments
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@ewfian
Copy link
Contributor

ewfian commented Oct 1, 2020 •
edited
Loading

  • Package Name: @azure/storage-blob
  • Package Version: 12.2.1
  • Operating system:
  • nodejs
    • version:
  • browser
    • name/version: AppleWebKit/605.1.15
  • typescript
    • version:
  • Is the bug related to documentation in

Describe the bug
The whitespace after colon in http header is optional. But the function parseHeaders in xhrHttpsClient.ts and and xhrHttpClient.ts requires spaces.

If no space after colon like this
image

The first char in value of header will be cut down.
image

It will makes header broken and some thing wrong.
image

The cordova-plugin-wkwebview-file-xhr is a library hosted by Oracle that helps Cordova users to solving CORS issue. It polyfilled the function XMLHttpRequest.getAllResponseHeaders() and wrapped http request. but the return value of polyfilled getAllResponseHeaders() have no space after colon. it makes @azure/storage-blob upload failed.
I have created an issue for Oracle oracle/cordova-plugin-wkwebview-file-xhr#68

But I think @azure/storage-blob also needs to improve to avoid such unsoundness.

To Reproduce
Steps to reproduce the behavior:

  1. setup a Cordova environment
  2. install the https://github.com/apache/cordova-plugin-wkwebview-engine
  3. install the https://github.com/oracle/cordova-plugin-wkwebview-file-xhr
  4. call uploadBrowserData()

Expected behavior
Uploaded without errors.
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Oct 1, 2020
@jeremymeng
Copy link
Member

However, the spec for getAllResponseHeaders() has 0x3A 0x20 between header name and header value.

@jeremymeng jeremymeng added Azure.Core Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Oct 1, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Oct 1, 2020
@jeremymeng
Copy link
Member

https://xhr.spec.whatwg.org/#the-getallresponseheaders()-method

@ewfian
Copy link
Contributor Author

ewfian commented Oct 1, 2020

Thanks for spec.

@ewfian ewfian mentioned this issue Oct 5, 2020
Open
@jeremymeng
Copy link
Member

@bterlson @xirzec do we want to stick to the xhr getAllResponseHeaders() spec, or relax a bit when parsing the headers?

@xirzec
Copy link
Member

xirzec commented Nov 18, 2020

What is gained by supporting a non-standard version of getAllResponseHeaders()? It seems like other JS libs could be equally confused by this, so the polyfill should be fixed to follow the guidelines.

@jeremymeng
Copy link
Member

@ewfian has opened an issue in the oracle/cordova-plugin-wkwebview-file-xhr repo. Closing this as we don't plan to do anything here.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xirzec @jeremymeng @ewfian