Page MenuHomeSoftware Heritage

Allow the webapp to retrieve the history file via a GET endpoint
ClosedPublic

Authored by vsellier on Apr 7 2021, 3:10 PM.

Diff Detail

Repository
rDCNT Archive counters
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 D5442 (id=19456)

Rebasing onto 1b262c37b7...

Current branch diff-target is up to date.
Changes applied before test
commit 010e70ef805b3c902a4b77b98b36266d75f532f2
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Apr 7 15:08:31 2021 +0200

    Allow the webapp to retrieve the history file via a GET endpoint
    
    Related to T3165

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

vlorentz added inline comments.
swh/counters/api/server.py
35–40

hmm... this means we'll get a 404 if there's an option missing in the config. I'd rather have an explicit error, even if it's a 500.

ardumont added inline comments.
swh/counters/tests/test_server.py
190

What happens if you are asking crap to the endpoint?

as in /counters_history/gimme-some-loving-with-some-inexistent-file ;)

;D (kaboom ;)

swh/counters/api/server.py
35–40

What happens if we install the route line 43 in this conditional here?

swh/counters/tests/test_server.py
190

/counters_history/../../../etc/passwd

swh/counters/tests/test_server.py
190

...or that ;)

/me notesm down @vlorentz in the naugthy list [1]

[1] https://xkcd.com/838/

swh/counters/api/server.py
35–40

What happens if we install the route line 43 in this conditional here?

I wrote that initially but make a rollback with no real reasons. I will try but it should be ok

swh/counters/tests/test_server.py
190

:)
ok for the missing file, I didn't find (I confess I didn't search too long) how to return a 404, I will check

For the ../etc/password example, the filename is checked to avoid that when it's called from the RPC, (the _validate_filename method on history,py)

for the get method, the .. are computed in the url so `/counters_history/../etc/passwd will be equivalent to /etc/password and return a 404

swh/counters/tests/test_server.py
190

notes* and naughty* (damn that's annoying to not be able to amend typos)

and great for the validate filename thingy, I had forgotten about it ;)

  • Return a 404 if the requested file does not exist
  • Use a fixture to configure the tests to make them easier to read

Build is green

Patch application report for D5442 (id=19461)

Rebasing onto 1b262c37b7...

Current branch diff-target is up to date.
Changes applied before test
commit f384b99fa4870925fe26a9962e5eda1f5b731361
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Apr 7 15:08:31 2021 +0200

    Allow the webapp to retrieve the history file via a GET endpoint
    
    Related to T3165

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

This revision is now accepted and ready to land.Apr 7 2021, 4:47 PM
vsellier removed a reviewer: ardumont.

Register the history endpoint only if the configuration is present

This revision now requires review to proceed.Apr 7 2021, 4:47 PM
This revision is now accepted and ready to land.Apr 7 2021, 4:48 PM

Build is green

Patch application report for D5442 (id=19462)

Rebasing onto 1b262c37b7...

Current branch diff-target is up to date.
Changes applied before test
commit 42157fd04885da0a2e0dfe78cfc5f293da99c7b3
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Apr 7 15:08:31 2021 +0200

    Allow the webapp to retrieve the history file via a GET endpoint
    
    Related to T3165

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