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

Improve code snipper HTTP #304

Open
jaroga86 opened this issue Sep 15, 2022 · 3 comments
Open

Improve code snipper HTTP #304

jaroga86 opened this issue Sep 15, 2022 · 3 comments

Comments

@jaroga86
Copy link

Describe the bug

When the HTTP code snippet is generated, it is generated with the first letter in uppercase and the format should be the same as in the header. A difference if the type CURL is selected maintains the correct format.

To Reproduce
Execute a example of request.
Clic in code snipper {}.
In code select HTTP.

Expected behavior
The expected behavior is to keep the same format as the header, as is done in the CURL.

Platform:

OS: Windows 10
vscode version: 1.66.2
extension version: 1.18.7

Example:

Screenshot type http:
image
Screenshot type curl:
image

best regards

@jaroga86
Copy link
Author

@dimitropoulos
Please advise whether the capitalization of HTTP headers is needed at httpsnippet.
As HTTP Headers are case sensitive, we think this transformation (toUpperCase()) should not be done. It is preventing us from using the httpsnippet with the expected result and causing header capitalization caused errors while using the http code generated by httpsnippet.
This is the code section we don't understand, and even there is commented that it is not required. Shouldn't it be deleted, then?
// Capitalize header keys, even though it's not required by the spec.
const keyCapitalized = key.toLowerCase().replace(/(^|-)(\w)/g, input => input.toUpperCase());
push(${keyCapitalized}: ${allHeaders[key]});

@erunion
Copy link
Contributor

erunion commented Oct 3, 2022

Please advise whether the capitalization of HTTP headers is needed at httpsnippet.

@jaroga86 HTTP headers are case-insensitive in HTTP/1.1:

Each header field consists of a name followed by a colon (":") and the 
field value. Field names are case-insensitive.

However in HTTP/2 they must be lowercased:

Just as in HTTP/1.x, header field names are strings of ASCII characters 
that are compared in a case-insensitive fashion.  However, header field 
names MUST be converted to lowercase prior to their encoding in HTTP/2.  
A request or response containing uppercase header field names MUST 
be treated as malformed.

@verhovsky
Copy link
Contributor

verhovsky commented Feb 25, 2023

The user wants you to remove this line

const keyCapitalized = key.toLowerCase().replace(/(^|-)(\w)/g, input => input.toUpperCase());

and just keep the header as-is presumably because whatever server they're using is expecting those custom headers to be lower case.

If you input "operation-date", it should output "operation-date", but in this case the HTTP target is needlessly capitalizing all headers, including that header name too, to Operation-Date. The curl target correctly preserves the case and outputs operation-date.

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

3 participants