Page MenuHomeSoftware Heritage
Differential D5487

identifiers: Fix parsing of SWHID qualifier value containing '='
ClosedPublic
Actions

Authored by anlambert on Apr 12 2021, 5:48 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDMOD74b024f5e0b8: identifiers: Fix parsing of SWHID qualifier value containing '='
Summary

According to the SWHID specification, it is not forbidden for a qualifier
value to contain a '=' character (for instance in origin URL).

So update parsing code to handle that special case.

Diff Detail

Repository
rDMOD Data model
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Apr 12 2021, 5:48 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 12 2021, 5:48 PM
anlambert added inline comments.Apr 12 2021, 5:49 PM
swh/model/tests/test_identifiers.py
1580–1585

I have a doubt whether the path percent encoding is expected or not here.

swh-public-ci added a comment.Apr 12 2021, 5:50 PM

Build is green

Patch application report for D5487 (id=19621)

Rebasing onto f2dba177ad...

First, rewinding head to replay your work on top of it...
Applying: identifiers: Fix parsing of SWHID qualifier value containing '='
Changes applied before test
commit 7ec3890a8466d1f25d89a8f575a9f76e19e2eb3c
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon Apr 12 17:44:20 2021 +0200

    identifiers: Fix parsing of SWHID qualifier value containing '='
    
    According to the SWHID specification, it is not forbidden for a qualifier
    value to contain a '=' character (for instance in origin URL).
    
    So update parsing code to handle that special case.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/311/ for more details.

anlambert mentioned this in D5484: assets/webapp-utils: Add SWHID validation in search inputs.Apr 12 2021, 5:50 PM
Harbormaster completed remote builds in B20651: Diff 19621.Apr 12 2021, 5:50 PM
anlambert requested review of this revision.Apr 12 2021, 5:50 PM
vlorentz requested changes to this revision.Apr 12 2021, 6:33 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/model/identifiers.py
1233–1234
swh/model/tests/test_identifiers.py
1580–1585

not expected, but accepted

This revision now requires changes to proceed.Apr 12 2021, 6:33 PM
anlambert added inline comments.Apr 12 2021, 6:50 PM
swh/model/identifiers.py
1233–1234

TIL the maxsplit option for split, thanks !

swh/model/tests/test_identifiers.py
1580–1585

Paths containing = character are indeed percent encoded in the SWHID copiable from the webapp: https://archive.softwareheritage.org/browse/directory/3ded8cb4d4770f9451b2f27e1576a8508737d8a7/?path=r%3D6

I guess we should leave it as it is.

anlambert updated this revision to Diff 19622.Apr 12 2021, 6:54 PM

Simplify code.

swh-public-ci added a comment.Apr 12 2021, 6:55 PM

Build is green

Patch application report for D5487 (id=19622)

Rebasing onto f2dba177ad...

First, rewinding head to replay your work on top of it...
Applying: identifiers: Fix parsing of SWHID qualifier value containing '='
Changes applied before test
commit cc5e73ab507bc7d287304787dcf3eef8c9317fad
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon Apr 12 17:44:20 2021 +0200

    identifiers: Fix parsing of SWHID qualifier value containing '='
    
    According to the SWHID specification, it is not forbidden for a qualifier
    value to contain a '=' character (for instance in origin URL).
    
    So update parsing code to handle that special case.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/312/ for more details.

Harbormaster completed remote builds in B20652: Diff 19622.Apr 12 2021, 6:55 PM
anlambert updated this revision to Diff 19623.Apr 12 2021, 7:01 PM

Use kwarg for readability

swh-public-ci added a comment.Apr 12 2021, 7:03 PM

Build is green

Patch application report for D5487 (id=19623)

Rebasing onto f2dba177ad...

First, rewinding head to replay your work on top of it...
Applying: identifiers: Fix parsing of SWHID qualifier value containing '='
Changes applied before test
commit 2b824e68b3605df89d5e6fff63470c813c1b9ba7
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon Apr 12 17:44:20 2021 +0200

    identifiers: Fix parsing of SWHID qualifier value containing '='
    
    According to the SWHID specification, it is not forbidden for a qualifier
    value to contain a '=' character (for instance in origin URL).
    
    So update parsing code to handle that special case.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/313/ for more details.

Harbormaster completed remote builds in B20653: Diff 19623.Apr 12 2021, 7:03 PM
anlambert added a comment.Apr 12 2021, 7:07 PM

In the webapp, origin URL with a = character in it also got percent encoded to avoid generating invalid browse URL: https://archive.softwareheritage.org/browse/origin/directory/?origin_url=https://cran.r-project.org/package%3Dtelegram.bot.

I am wondering if we should not also force to percent encode = in qualifier value as we do for ;;

vlorentz added a comment.Apr 12 2021, 8:34 PM

Too late for that; we can't know if there are SWHIDs in the wild with unescaped equal signs in them.

anlambert added a comment.Apr 13 2021, 10:50 AM
In D5487#139263, @vlorentz wrote:

Too late for that; we can't know if there are SWHIDs in the wild with unescaped equal signs in them.

Ack, good news is that diff will fix SWHID resolution in the webapp when the = character in an origin URL is not percent encoded (currently it fails as expected).

anlambert updated this revision to Diff 19638.Apr 13 2021, 2:03 PM

Rebase

swh-public-ci added a comment.Apr 13 2021, 2:04 PM

Build is green

Patch application report for D5487 (id=19638)

Rebasing onto 15d5bab512...

Current branch diff-target is up to date.
Changes applied before test
commit 74b024f5e0b862e8cff12c62406d06b22d258c84
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon Apr 12 17:44:20 2021 +0200

    identifiers: Fix parsing of SWHID qualifier value containing '='
    
    According to the SWHID specification, it is not forbidden for a qualifier
    value to contain a '=' character (for instance in origin URL).
    
    So update parsing code to handle that special case.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/315/ for more details.

Harbormaster completed remote builds in B20666: Diff 19638.Apr 13 2021, 2:04 PM
vlorentz accepted this revision.Apr 13 2021, 3:12 PM
This revision is now accepted and ready to land.Apr 13 2021, 3:12 PM
Closed by commit rDMOD74b024f5e0b8: identifiers: Fix parsing of SWHID qualifier value containing '=' (authored by anlambert). · Explain WhyApr 13 2021, 3:27 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDMOD74b024f5e0b8: identifiers: Fix parsing of SWHID qualifier value containing '='.

Revision Contents
Changeset List

PathSize
swh/
model/
4 lines
tests/
14 lines

Diff 19643

View Options

swh/model/identifiers.py

Loading...
View Options

swh/model/tests/test_identifiers.py

Loading...
About · Developer information · Legal