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.

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

guarantee correct URLs #992

Closed
jkowalleck opened this issue Dec 1, 2023 · 4 comments · Fixed by #996
Closed

guarantee correct URLs #992

jkowalleck opened this issue Dec 1, 2023 · 4 comments · Fixed by #996
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jkowalleck
Copy link
Member

jkowalleck commented Dec 1, 2023

caused by CycloneDX/cyclonedx-webpack-plugin#1239 (comment)

An issue exists, where (invalid) URLs like https://github.com/cssinjs/jss/issues/new?title=[jss-plugin-camel-case] cause trouble.
characters [] are invalid characters to URL standards. they must be url encoded %5B%5D.

possible fix can be done in this library, on normalization time (not in the model).
would not be the first time to fix this ...
see https://github.com/search?q=repo%3ACycloneDX%2Fcyclonedx-php-library+%255B&type=code


similar to CycloneDX/cyclonedx-php-library#35

have all the XML strings that are anyURI somehow fixed before rendering the XML/JSON.
affected elements:

  • component.purl
  • license.url
  • externalReterence.url
  • and so on ...

according to XML spec the anyURI needs to conform to https://www.ietf.org/rfc/rfc2396.txt

 * @see http://www.w3.org/TR/xmlschema-2/#anyURI
 * @see http://www.datypic.com/sc/xsd/t-xsd_anyURI.html


    /* URIs require that some characters be escaped with their hexadecimal Unicode code point preceded by the %
     * character. This includes non-ASCII characters and some ASCII characters, namely control characters, spaces,
     * and the following characters (unless they are used as deliimiters in the URI): <>#%{}|\^`.
     * [...]
     * The only values that are not accepted are ones that make inappropriate use of reserved characters, such as ones that contain multiple # characters or have % characters that are not followed by two hexadecimal digits.
     * -- as of http://www.datypic.com/sc/xsd/t-xsd_anyURI.html
     */
@jkowalleck
Copy link
Member Author

jkowalleck commented Dec 1, 2023

@mLuca ,
do you want to give it a try and implement something that fixes those URLs?

url fixes SHOULD happen on (xml-/json-)normalization-time, not in the model.

see as inspiration: https://github.com/search?q=repo%3ACycloneDX%2Fcyclonedx-php-library+%255B&type=code

@jkowalleck jkowalleck added the good first issue Good for newcomers label Dec 1, 2023
@jkowalleck jkowalleck pinned this issue Dec 1, 2023
@jkowalleck jkowalleck added bug Something isn't working and removed enhancement New feature or request labels Dec 2, 2023
@mLuca
Copy link
Contributor

mLuca commented Dec 4, 2023

@jkowalleck I can have a look today.

@mLuca
Copy link
Contributor

mLuca commented Dec 4, 2023

@jkowalleck I had a look at the code base and grasped a bit of typescript on the way.
I have written a helper method to be used. At least it doesn't seem to do any harm. Before I go on to write tests and such it would be nice to have a review on this if the direction I am going is ok.

I am new to GitHub, OSS development and such. I don't see how I can create a bugfix-branch to push my changes to? Also I didn't find any styleguide on how to name branches (e.g. "bugfix/992-assert-correct-url" ?)

@jkowalleck
Copy link
Member Author

Before I go on to write tests and such it would be nice to have a review on this if the direction I am going is ok.

sure thing. just open a pullrequest and i will have a look.

since you do not have write permission to this very repository, you would create a pullrequest from a fork: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

Since you would create the branch in your fork, I don't mind the branch name.
Regarding contribution guidelines for this very project, read https://github.com/CycloneDX/cyclonedx-javascript-library/blob/main/CONTRIBUTING.md

@jkowalleck jkowalleck added enhancement New feature or request and removed bug Something isn't working labels Dec 6, 2023
@jkowalleck jkowalleck changed the title assert correct URLs guarantee correct URLs Dec 11, 2023
jkowalleck added a commit that referenced this issue Dec 11, 2023
Serialization/normalization guarantees valid URI values according to JSON/XML specification

fixes #992
---------

Signed-off-by: mzl2fe <luca.mazzon@etas.com>
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
Co-authored-by: mzl2fe <luca.mazzon@etas.com>
Co-authored-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck unpinned this issue Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants