Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: encoding of special chars when encoding to string #37

Merged
merged 2 commits into from
Mar 7, 2023
Merged

fix: encoding of special chars when encoding to string #37

merged 2 commits into from
Mar 7, 2023

Conversation

jkowalleck
Copy link
Contributor

@jkowalleck jkowalleck commented Mar 3, 2023 •
edited

  • tests for proper encoding when toString() is called
  • fixed some encoding issues

@jkowalleck jkowalleck marked this pull request as draft March 3, 2023 10:30
@jkowalleck
fix: encoding of special chars when encoding to string
49fcdf6 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck marked this pull request as ready for review March 3, 2023 10:51
@jkowalleck
Copy link
Contributor Author

@steven-esser could you review?

steven-esser
steven-esser previously approved these changes Mar 3, 2023
View reviewed changes
Copy link
Collaborator

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

@jkowalleck This looks great. Thank you for your contribution!

@jkowalleck
docs: changelog for package-url#37
f18114f 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@steven-esser steven-esser merged commit d7cbfe6 into package-url:master Mar 7, 2023
@steven-esser
Copy link
Collaborator

@jkowalleck v1.0.1 released to npm with your fixes: https://www.npmjs.com/package/packageurl-js/v/1.0.1

Thanks again!

@anuccio1
Copy link

anuccio1 commented May 12, 2023 •
edited

Just a heads up that this was actually a breaking change for my team. We aren't blocked per-say, just now locked into version 1.0.0.

Example Failing test:

AssertionError: expected 'pkg:maven/org.wso2.carbon.identity.fr…' to equal 'pkg:maven/org.wso2.carbon.identity.fr…'
      + expected - actual

      -pkg:maven/org.wso2.carbon.identity.framework/org.wso2.carbon.identity.user.profile.ui@5.12.21?repository_url=https%3A//maven.wso2.org/nexus/content/groups/wso2-public
      +pkg:maven/org.wso2.carbon.identity.framework/org.wso2.carbon.identity.user.profile.ui@5.12.21?repository_url=https://maven.wso2.org/nexus/content/groups/wso2-public

Perhaps we can add in an option to not encode ?

@jkowalleck
Copy link
Contributor Author

https://github.com/package-url/purl-spec/blob/master/PURL-SPECIFICATION.rst#rules-for-each-purl-component
-> section "qualifiers"

A value must be a percent-encoded string

satanshiro pushed a commit to satanshiro/packageurl-js that referenced this pull request Oct 30, 2023
@jkowalleck @satanshiro
fix: encoding of special chars when encoding to string (package-url#37)
d174684 
* fix: encoding of special chars when encoding to string
* docs: changelog for package-url#37

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steven-esser steven-esser steven-esser approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jkowalleck @steven-esser @anuccio1