Fix #1 #17
Fix #1 #17
Conversation
| stopServer(t) | ||
| t.doesNotThrow(ra.write.bind(ra, 10, 'some-data', function () { | ||
| stopServer(t) | ||
| })) |
tests/valid-url.test.js Outdated
| }) | ||
| | ||
| test('parseUrl assumes http with path', (t) => { | ||
| t.same(parseUrl('example.com/foo/bar.html'), 'http://example.com/foo/bar.html') |
There was a problem hiding this comment.
should we assume https:// by default? Perhaps open another issue to enable falling back to http. This will ensure more secure connections are preferred always.
There was a problem hiding this comment.
What happens if the certificate is not valid? A correct server implementation will redirect http to https when available IMO.
lib/url.js Outdated
| return ['http:', 'https:'].includes(parsed.protocol) | ||
| } | ||
| | ||
| module.exports.parseUrl = function (str) { |
There was a problem hiding this comment.
This is not really a parse function, so the name is slightly misleading. Perhaps appendUrlProtocol is a more descriptive name.
There was a problem hiding this comment.
replaced by prependUrlProtocol
lib/url.js Outdated
| module.exports.validUrl = function (str) { | ||
| if (typeof str !== 'string') return false | ||
| var parsed = url.parse(str) | ||
| return ['http:', 'https:'].includes(parsed.protocol) |
There was a problem hiding this comment.
It might be worth using regex for this check, just to avoid needing to use polyfills for this to work in IE.
| Thanks again. This looks great. We can maybe add an option later to prefer https or http as default. |
In this case user should just add the protocol :D. |
😆 You are too right. |
3 participants
No description provided.