fixes #15363 - construct correct templates query for new hosts #3586
Conversation
domcleal
force-pushed
the
15363-resolve-urls
branch
from
0487829
to
59dc9dd
Compare
| @@ -255,7 +255,7 @@ function template_info(div, url) { | |||
| // Use a post to avoid request URI too large issues with big forms | |||
| $.ajax({ | |||
| type: "POST", | |||
| url: url + "&provisioning=" + build, | |||
| url: url + ((url.indexOf('?') == -1) ? '?' : '&') + "provisioning=" + build, | |||
There was a problem hiding this comment.
nitpick: isnt it enough to find the first usage of ? vs anywhere in the string? for example:
(new URL(url)).search[0] === '?'There was a problem hiding this comment.
I can't tell what advantage that provides over the current implementation. Are you suggesting it needs to handle question marks in the path or host portions of the URL?
There was a problem hiding this comment.
again - nitpick, however, i believe this is more "correct" e.g. your code will return true if url has a question mark in the middle of the string.
There was a problem hiding this comment.
url.indexOf('?') should return false if there's a question mark in the url string, as that would be interpreted as a query string by the server. If the path component of the URL was intended to contain a question mark, it would be URL-encoded and not picked up by indexOf.
There was a problem hiding this comment.
my point is that since this bug is in contruction the url, we can be a bit more specific. for example:
url="http://localhost:3000/hosts/template_used?x=1?&=2"
url.indexOf('?')
41
url="http://localhost:3000/hosts/template_used&x=1?y=2"
url.indexOf('?')
45I realize that my solution is not useful either in this case:
when testing with URL
"http://localhost:3000/hosts/template_used&x=1?y=2"
(new URL(url)).search
"?y=2"I guess this not worth the time, merging :)
No description provided.