Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

URI encode parts of the extension download URL #59

Merged
merged 2 commits into from
Aug 12, 2014
Merged

Conversation

ingorichter
Copy link
Contributor

This fixes adobe/brackets#8692.
The extension name and the extension version will now be URI encoded and all URL unsafe characters are replaced by their % encoded entities.


it("should return the formatted url with proper url encoding", function () {
var formattedURL = registryUtils.formatDownloadURL("http://localhost:1234", "jasonsanjose.brackets-sass", "0.4.1+sha.fc425b5");
expect("http://localhost:1234/jasonsanjose.brackets-sass/jasonsanjose.brackets-sass-0.4.1%2Bsha.fc425b5.zip").toBe(formattedURL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is backwards... it should be expect(formattedURL).toBe("http://...") (expect takes the "actual" value)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and No. The output reads nicer when writing it this way. ;-) I'm going to change it to avoid confusion.

@dangoor dangoor self-assigned this Aug 11, 2014
@@ -10,7 +10,7 @@
<tr>
<td {{#if canAdmin}}class="info"{{/if}}></td>
<td>
<a href="{{../repositoryBaseURL}}/{{metadata.name}}/{{metadata.name}}-{{metadata.version}}.zip">
<a href="{{formatDownloadURL ../repositoryBaseURL metadata.name metadata.version}}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use 3 braces to not escape the already escaped URL again.

@dangoor
Copy link
Contributor

dangoor commented Aug 12, 2014

Looks good. Merging.

dangoor added a commit that referenced this pull request Aug 12, 2014
URI encode parts of the extension download URL
@dangoor dangoor merged commit c04c2d5 into master Aug 12, 2014
@dangoor dangoor deleted the ingo/fix-8692 branch August 12, 2014 19:25
@ingorichter
Copy link
Contributor Author

Closing. This got already deployed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extension Manager returns 404 error for when extension version contains "+"
3 participants