Uh oh!
There was an error while loading. Please reload this page.
feat: adding a new harIsAlreadyEncoded option - #46
Conversation
| const fetch = require('node-fetch'); | ||
| const url = 'http://mockbin.com/har'; | ||
| const options = {method: 'POST', headers: {cookie: 'foo=bar; bar=baz; '}}; |
There was a problem hiding this comment.
Cool side effect of this work is now cookies for node-fetch won't have an unnecessary space at the end.
| if (Object.keys(source.headersObj).length) { | ||
| reqOpts.headers = source.headersObj | ||
| if (Object.keys(source.allHeaders).length) { | ||
| reqOpts.headers = source.allHeaders |
There was a problem hiding this comment.
What are the implications of consuming allHeaders instead of headersObj?
There was a problem hiding this comment.
allHeaders contains the cookie header if it's present, headersObj is a k/v store of headers from the HAR excluding cookie because that's dynamically generated from the cookies array in the HAR.
There was a problem hiding this comment.
And by changing this to allHeaders this target no longer needs to combine its own cookie data into a cookie header because that's already been done in src/index.js.
With the work done in readmeio/oas-to-har#17 to ensure that query parameters are always properly encoded it was discovered that cookie parameters were being double encoded: once in
oas-to-harand again here. Since this is obviously bad, I've refactored our existing customescapeQueryStringsoption to beharIsAlreadyEncoded(defaulting to false) so that we can now control both query string and cookie encoding behavior.In the process of doing this I've slightly refactored a couple targets to no longer do their own cookie encoding as it's already being done in the
HTTPSnippetconstructor when it builds up a list ofallHeaders. This refactor also allows us to not have to pass an option toHTTPSnippetand also to the target itself to control encoding handling because the rootHTTPSnippetclass does not currently have any way of passing down options to targets.