Skip to content

Conversation

@kou
Copy link
Contributor

@kou kou commented Nov 22, 2023

If a SubmitBatch request has a HTTP header that has a HTTP header
delimiter (":") in its value, the SubmitBatch request is failed as
"400 One of the request inputs is not valid."

For example, if user-agent header is azsdk-cpp-storage-blobs/12.10.0-beta.1 (Darwin 23.1.0 arm64 Darwin Kernel Version 23.1.0: Mon Oct 9 21:28:12 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T8103), all SubmitBatch() requests are failed.

@kou kou force-pushed the submit-batch-parse-header branch 2 times, most recently from a7122fa to 7c2d4ff Compare November 22, 2023 05:00
keepAliveOptions: { enable: false },
// Make sure HTTP headers are parsed properly with many HTTP
// header delimiters.
userAgentOptions: { userAgentPrefix: "include:HTTP:header:delimiter " }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... It seems that this doesn't change user-agent header in SubmitBatch request...

Could someone advice me how to write a test for this case? I want to add header-name: value:value header to SubmitBatch request...

Copy link
Member

@EmmaZhu EmmaZhu Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JS SDK doesn't support to set headers in sub requests.
Seems you encountered the issue when you are using CPP sdk?
It should be fine if azurite can pass manual test. Could you share your repro project to me? Then I can run a manual test.

Copy link
Contributor Author

@kou kou Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.
Yes, we're using azure-sdk-for-cpp.
apache/arrow#38793 is the project (and the change) that encounters this problem.
But I disabled the test that encountered this problem for now: https://github.com/apache/arrow/pull/38793/files#diff-7d2cbf9c6abf1ee980b8cf5a79e222543c17b3d93b12c4c0c9fa123b1bb200b6R601-R604

Here is the test function that encountered this problem:
https://github.com/apache/arrow/pull/38793/files#diff-7d2cbf9c6abf1ee980b8cf5a79e222543c17b3d93b12c4c0c9fa123b1bb200b6R600-R624

Here is the code that creates a SubmitBatch request:
https://github.com/apache/arrow/pull/38793/files#diff-3bd20682a329dbf3fde0bed6310f07f71cd25312b24d4baa1de5764ebdc8c0acR1020-R1081

If you need more information, please let me know.

Anyway, I'll revert the blobbatch.test.ts change.

@blueww
Copy link
Member

blueww commented Nov 22, 2023

@EmmaZhu

Would you please help to look at the fix for blob batch code?

If a SubmitBatch request has a HTTP header that has a HTTP header
delimiter (":") in its value, the SubmitBatch request is failed as
"400 One of the request inputs is not valid."

For example, if `user-agent` header is
`azsdk-cpp-storage-blobs/12.10.0-beta.1 (Darwin 23.1.0 arm64 Darwin
Kernel Version 23.1.0: Mon Oct 9 21:28:12 PDT 2023;
root:xnu-10002.41.9~6/RELEASE_ARM64_T8103)`, all `SubmitBatch()`
requests are failed.
@kou kou force-pushed the submit-batch-parse-header branch from 7c2d4ff to 15128c7 Compare November 23, 2023 14:37
@EmmaZhu
Copy link
Member

EmmaZhu commented Dec 19, 2023

/check-enforcer reset

@EmmaZhu
Copy link
Member

EmmaZhu commented Dec 19, 2023

I've manually tested the fixed. It works well with a header like testheader: testvalue:test:value in sub requests. Will approve the PR.
@blueww , do you have other concerns?

@EmmaZhu
Copy link
Member

EmmaZhu commented Dec 19, 2023

/check-enforcer evaluate

@EmmaZhu
Copy link
Member

EmmaZhu commented Dec 19, 2023

/check-enforcer reset

@EmmaZhu
Copy link
Member

EmmaZhu commented Dec 20, 2023

/check-enforcer evaluate

@felipecrv
Copy link

I started hitting this in my unit tests as well now.

@EmmaZhu EmmaZhu merged commit f005399 into Azure:main Dec 26, 2023
@kou kou deleted the submit-batch-parse-header branch December 27, 2023 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants