Repository navigation
Allow form.submit with url string param to use https - #249
Conversation
|
|
2 similar comments
|
Sorry for a messy PR. Fixed my test to pass on node 0.10.x |
2 similar comments
|
Squashed my commits into one for nicer history |
alexindigo
left a comment
alexindigo
left a comment
There was a problem hiding this comment.
Thank you for taking care of it. I have couple suggestions.
| req = form.submit('http://localhost:' + common.port + '/path', function() {}); | ||
| assert.strictEqual(req.path, '/path'); | ||
| assert.ok(req.agent instanceof http.Agent, 'req.agent instanceof http.Agent'); | ||
| assert.strictEqual(req.getHeader('Host'), 'localhost:' + common.port); |
There was a problem hiding this comment.
Can you add similar check to the https counter part?
And maybe try with default ports, since you're going through with the request anyway.
Sounds good?
There was a problem hiding this comment.
Yep. I'll push fixes once the comment above is resolved.
|
|
||
| // Basic parsing | ||
| req = form.submit('http://localhost:' + common.port + '/path', function() {}); | ||
| assert.strictEqual(req.path, '/path'); |
There was a problem hiding this comment.
I'm not sure, what are you referring to. The /path assertation?
|
@alexindigo sorry for the delay. I improved the tests a bit. |
|
👍 |
|
Is there a plan to release this? |
|
Thank you @vmosyaykin |
I want to be able to pass a string starting with
https:toform.submit, and I expect the form to be submitted over https.url.parsereturns theprotocolproperty, however previously it was ignored.I also added some tests, and would like to hear some comments on them. I had to write them a bit differently from the others, because you can't pass options for self-signed certs when using string as first parameter to
form.submit.