Page MenuHomeSoftware Heritage

assets/webapp-utils: Add SWHID validation in search inputs
ClosedPublic

Authored by anlambert on Apr 12 2021, 3:25 PM.

Details

Summary

Check a SWHID entered in a search input is valid by sending a request
to the resolve endpoint of the Web API.

If the SWHID is not valid, precise parsing error will be extracted from
the Web API response and displayed in the Web UI to indicate the search
form can not be validated.

As the resolve endpoint will be more requested, we should change its rate limit settings
to allow more requests (120 per minute for instance).


Related to T3247

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D5484 (id=19612)

Rebasing onto 7131f6cd49...

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

    assets/webapp-utils: Automatically fix malformed SWHID in search inputs
    
    Handle malformed SWHID entered in search input due to an invalid trailing
    slash, for instance when copying the SWHID from its browse URL.
    
    Remove the erroneous trailing slash in that case and update the input.
    
    Related to T3234

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

Are you planning to add a warning in a future diff?

imo the new code in webapp-utils.js should be in its own function, because the $(document).ready anonymous function is starting to be very long

assets/src/bundles/webapp/webapp-utils.js
182

could you document the motivation? (you can copy-paste the example in T3234)

194

Doesn't work if there is = in the value

This revision now requires changes to proceed.Apr 12 2021, 3:55 PM

Are you planning to add a warning in a future diff?

Nope, I do not see the interest. Such copy paste errors will be pretty rare imho.

But we could add a SWHID validator for the search input, what do you think ?

imo the new code in webapp-utils.js should be in its own function, because the $(document).ready anonymous function is starting to be very long

Ack

assets/src/bundles/webapp/webapp-utils.js
182

I will add the URL of the task in the comment.

194

Based on qualifiers parsing code in swh-model, having a = character in qualifier value will raise a ValidationError.

I guess I can check the size of the split and abort if is not equal to 2.

Build is green

Patch application report for D5484 (id=19615)

Rebasing onto 7131f6cd49...

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

    assets/webapp-utils: Automatically fix malformed SWHID in search inputs
    
    Handle malformed SWHID entered in search input due to an invalid trailing
    slash, for instance when copying the SWHID from its browse URL.
    
    Remove the erroneous trailing slash in that case and update the input.
    
    Related to T3234

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

assets/src/bundles/webapp/webapp-utils.js
194

then it's a bug in swh-model. From the SWHID spec:

<context_qualifier> ::=
    <origin_ctxt>
  | <visit_ctxt>
  | <anchor_ctxt>
  | <path_ctxt>
  ;
<origin_ctxt> ::= "origin" "=" <url_escaped> ;
<path_ctxt> ::= "path" "=" <path_absolute_escaped> ;

where

    <url_escaped> is a RFC 3987 IRI

in either case all occurrences of ; (and %, as required by the RFC) have been percent-encoded (as %3B and %25 respectively). Other characters can be percent-encoded, e.g., to improve readability and/or embeddability of SWHID in other contexts.

and from said RFC:

ipath-absolute = "/" [ isegment-nz *( "/" isegment ) ]
isegment       = *ipchar
ipchar         = iunreserved / pct-encoded / sub-delims / ":"
               / "@"
sub-delims     = "!" / "$" / "&" / "'" / "(" / ")"
               / "*" / "+" / "," / ";" / "="
194

forgot this line:

<path_absolute_escaped> is an <ipath-absolute> from RFC 3987, and
assets/src/bundles/webapp/webapp-utils.js
194

Does it mean that we should split the qualifier at the first = character or percent encode it in qualifier value ?

assets/src/bundles/webapp/webapp-utils.js
194

the former

assets/src/bundles/webapp/webapp-utils.js
194

Ack, will fix the issue in swh-model first and update that diff afterwards.

assets/src/bundles/webapp/webapp-utils.js
194

Thinking it back, the proper way to avoid sending a malformed SWHID to the resolver is to implement a form validator
as it is already done for the save code now form (try to save a malformed URL and form will not be validated, details
of the validation issue are then displayed below the search input).

This way we will have an error displayed and the resolve request will not be send.
So the idea will be to implement a SWHID validator in Javascript to see if the input
SWHID respects the specification. An extra processing for erroneous trailing slash will
also be added with a proper error message displayed in that case.

Update: Validate SWHID and report any parsing errors instead of autofixing it.

anlambert retitled this revision from assets/webapp-utils: Automatically fix malformed SWHID in search inputs to assets/webapp-utils: Add SWHID validation in search inputs.Apr 13 2021, 5:07 PM
anlambert edited the summary of this revision. (Show Details)
anlambert edited the summary of this revision. (Show Details)

Build is green

Patch application report for D5484 (id=19648)

Rebasing onto 78a038b139...

First, rewinding head to replay your work on top of it...
Applying: assets/webapp-utils: Add SWHID validation in search inputs
Changes applied before test
commit d42538d7bd363faba738ebb7b6eb37a201b5d196
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Mon Apr 12 15:15:44 2021 +0200

    assets/webapp-utils: Add SWHID validation in search inputs
    
    Check a SWHID entered in a search input is valid by sending a request
    to the resolve endpoint of the Web API.
    
    If the SWHID is not valid, precise parsing error will be extracted from
    the Web API response and displayed in the Web UI to indicate the search
    form can not be validated.
    
    Related to T3234

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

Build is green

Patch application report for D5484 (id=19685)

Rebasing onto 6fa50083a1...

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

    assets/webapp-utils: Add SWHID validation in search inputs
    
    Check a SWHID entered in a search input is valid by sending a request
    to the resolve endpoint of the Web API.
    
    If the SWHID is not valid, precise parsing error will be extracted from
    the Web API response and displayed in the Web UI to indicate the search
    form can not be validated.
    
    Related to T3247

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

Nice! Why the green labels/checkboxes/dropdowns?

Nice! Why the green labels/checkboxes/dropdowns?

Because bootstrap apply the validation process on the whole form and all its elements.

This is disturbing at first sight but nevertheless it emphasizes the invalid input field.

I can try to validate only the input element but code will be more hackish.

Build is green

Patch application report for D5484 (id=19786)

Rebasing onto b095564e54...

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

    assets/webapp-utils: Add SWHID validation in search inputs
    
    Check a SWHID entered in a search input is valid by sending a request
    to the resolve endpoint of the Web API.
    
    If the SWHID is not valid, precise parsing error will be extracted from
    the Web API response and displayed in the Web UI to indicate the search
    form can not be validated.
    
    Related to T3247

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

This revision is now accepted and ready to land.Apr 19 2021, 1:00 PM