add trailing slash to amo-base-url & enforce within submit-addon Client #2621
Conversation
src/util/submit-addon.js
Outdated
| @@ -90,6 +90,9 @@ export default class Client { | |||
| userAgentString, | |||
| }: ClientConstructorParams) { | |||
| this.apiAuth = apiAuth; | |||
| if (!baseUrl.href.endsWith('/')) { | |||
There was a problem hiding this comment.
If the URL somehow contains a ? or # but not a /, then this would not work as desired.
This would fix the issue:
if (!baseUrl.pathname.endsWith('/')) {`
baseUrl = new URL(baseUrl);
baseUrl.pathname += "/";
}P.S. The query and ref will be stripped by new URL('addons/', baseUrl) anyway, so an alternative is to assert that baseUrl.search and baseUrl.hash are both empty.
There was a problem hiding this comment.
The fix suggestion sounds good. I'm not sure it's worth the extra effort to assert on baseUrl.search and baseUrl.hash though - the code is too specialised to the AMO API here, so trying to support another API that has a different url style is out of scope. (My vote is just to ignore it and let them be stripped)
There was a problem hiding this comment.
757f47a is your fix(ish)
|
@Rob--W if that looks good to you, please merge :) (so that I'll know that you saw both this message and the updated patch) |
fixes #2579