Page MenuHomeSoftware Heritage
Differential D5501

add an anti-Dos limit for edges traversed as a query parameter
ClosedPublic
Actions

Authored by Hakimb on Apr 13 2021, 5:36 PM.

Details

Reviewers
vlorentz
seirl
Group Reviewers
Reviewers
Maniphest Tasks
T3161: graph service: add anti-DoS limit on the number of edges traversed
Commits
rDGRPHe8f052a34003: Add an anti DoS limit for edges traversed as a query parameter.
Summary

Now we have a 'max_edges' query argument to limit the number of edges traversed during a visit on the graph, 'max_edges' has no default value nor upper bound.

related to T3161.

Diff Detail

Repository
rDGRPH Compressed graph representation
Branch
anti-ddos-visit
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20908
Build 32445: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 32444: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
2 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.graph.tests.test_graph::test_leaves
graph = <swh.graph.graph.Graph object at 0x7f8ecf5ada58> def test_leaves(graph):
1 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.graph.tests.test_graph::test_visit_nodes
graph = <swh.graph.graph.Graph object at 0x7f8ecf5ada58> def test_visit_nodes(graph):
14 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.graph.tests.test_api_client::test_count
14 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.graph.tests.test_api_client::test_leaves
7 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.graph.tests.test_api_client::test_neighbors
View Full Test Results (2 Failed · 31 Passed · 2 Skipped)

Event Timeline

Hakimb created this revision.Apr 13 2021, 5:36 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 13 2021, 5:36 PM
swh-public-ci added a comment.Apr 13 2021, 5:38 PM

Build is green

Patch application report for D5501 (id=19651)

Rebasing onto 4f751998c6...

Current branch diff-target is up to date.
Changes applied before test
commit 5c2aecebb4d4ea42280e9a934281e70751a91e71
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti Ddos limit for edges traversed as a query parameter

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

Harbormaster completed remote builds in B20679: Diff 19651.Apr 13 2021, 5:38 PM
Hakimb requested review of this revision.Apr 13 2021, 5:38 PM
vlorentz added a subscriber: vlorentz.Apr 13 2021, 6:37 PM

Instead of making max_edges an object and needing an extra boolean; what about making it an unsigned long (or whatever the java equivalent is), with the value 0 meaning there is no limit?
This way, the syntax is more concise, no need for Long.valueOf(max_edges.toString()) to cast the number, and types are strictly enforced.

Also, nitpick about the commit message: it's an anti-DoS, not anti-DDoS (the first D in DDoS implies it only works maliciously by having someone access it from many locations; but the "bug" you are fixing can be triggered from a single client)

vlorentz requested changes to this revision.Apr 13 2021, 6:37 PM
This revision now requires changes to proceed.Apr 13 2021, 6:37 PM
Hakimb updated this revision to Diff 19696.Apr 14 2021, 3:56 PM

'max_edges' have now a default value (0 for no limit) which allow to simplify the java code

Hakimb retitled this revision from add an anti Ddos limit for edges traversed as a query parameter to add an anti-Dos limit for edges traversed as a query parameter.Apr 14 2021, 3:58 PM
swh-public-ci added a comment.Apr 14 2021, 4:16 PM

Build was aborted

Patch application report for D5501 (id=19696)

Could not rebase; Attempt merge onto 4f751998c6...

Updating 4f75199..727e0b6
Fast-forward
 .../java/org/softwareheritage/graph/Entry.java     | 12 +++----
 .../java/org/softwareheritage/graph/Traversal.java | 37 ++++++++++++++++------
 swh/graph/backend.py                               | 18 ++++++++---
 swh/graph/client.py                                | 12 +++----
 swh/graph/graph.py                                 | 15 +++++----
 swh/graph/server/app.py                            | 31 ++++++++++++++----
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++
 7 files changed, 109 insertions(+), 39 deletions(-)
Changes applied before test
commit 727e0b6c6c40958ad406d08f6b41433a24749c60
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 14 15:53:54 2021 +0200

    add an anti-Dos limit for edges traversed as a query parameter

commit 5c2aecebb4d4ea42280e9a934281e70751a91e71
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti Ddos limit for edges traversed as a query parameter

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/77/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/77/console

Harbormaster failed remote builds in B20725: Diff 19696!Apr 14 2021, 4:16 PM
vlorentz added a comment.Apr 15 2021, 10:12 AM

It looks like your diff only includes changes since my comment, instead of all the changes.

I'm guessing you have two commits and you only included the last one

Hakimb added a comment.Apr 15 2021, 12:03 PM

I'm not sure how to fix this problem. I just update the code and used "arc diff update..." etc

vlorentz added a comment.Apr 15 2021, 1:22 PM

git rebase -i master, see https://www.git-tower.com/learn/git/faq/git-squash/

Hakimb updated this revision to Diff 19783.Apr 16 2021, 11:29 AM

rebase

swh-public-ci added a comment.Apr 16 2021, 11:50 AM

Build was aborted

Patch application report for D5501 (id=19783)

Could not rebase; Attempt merge onto e86a39d9dc...

Updating e86a39d..00b3fbf
Fast-forward
 .../java/org/softwareheritage/graph/Entry.java     | 12 +++----
 .../java/org/softwareheritage/graph/Traversal.java | 37 ++++++++++++++++------
 swh/graph/backend.py                               | 18 ++++++++---
 swh/graph/client.py                                | 12 +++----
 swh/graph/graph.py                                 | 15 +++++----
 swh/graph/server/app.py                            | 31 ++++++++++++++----
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++
 7 files changed, 109 insertions(+), 39 deletions(-)
Changes applied before test
commit 00b3fbf60b22f0b1547fbc5c22115da931ec99ed
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 14 15:53:54 2021 +0200

    add an anti-Dos limit for edges traversed as a query parameter

commit 64202521ae2a2ff6b5881aa645c1a60420d8c317
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti Ddos limit for edges traversed as a query parameter

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/79/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/79/console

Harbormaster failed remote builds in B20796: Diff 19783!Apr 16 2021, 11:50 AM
Hakimb updated this revision to Diff 19791.Apr 16 2021, 12:50 PM

fix + rebase

seirl added a subscriber: seirl.Apr 16 2021, 1:09 PM

It looks to me like this would be simpler if max_edges was given as a parameter to Traversal, since it's common to most methods. Would that work?

swh-public-ci added a comment.Apr 16 2021, 1:11 PM

Build was aborted

Patch application report for D5501 (id=19791)

Could not rebase; Attempt merge onto e86a39d9dc...

Updating e86a39d..ea69d02
Fast-forward
 .../java/org/softwareheritage/graph/Entry.java     | 12 +++----
 .../java/org/softwareheritage/graph/Traversal.java | 37 ++++++++++++++++------
 swh/graph/backend.py                               | 18 ++++++++---
 swh/graph/client.py                                | 12 +++----
 swh/graph/graph.py                                 | 15 +++++----
 swh/graph/server/app.py                            | 31 ++++++++++++++----
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++
 7 files changed, 109 insertions(+), 39 deletions(-)
Changes applied before test
commit ea69d027349e63569506fd3488750b14bb71e323
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 16 12:48:51 2021 +0200

    rebase + fix + merge

commit 64202521ae2a2ff6b5881aa645c1a60420d8c317
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti Ddos limit for edges traversed as a query parameter

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/80/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/80/console

Harbormaster failed remote builds in B20804: Diff 19791!Apr 16 2021, 1:11 PM
vlorentz added a comment.Apr 16 2021, 1:30 PM

@Hakimb You rebase is still incorrect. If you look at the content of the diff, there is still some of your initial work in red.

And the commit message is now "rebase + fix + merge" (see the "Commits" tab in "Revision Contents")

Hakimb updated this revision to Diff 19792.Apr 16 2021, 2:04 PM

add an anti-Dos limit for edges traversed as a query parameter

Harbormaster failed remote builds in B20805: Diff 19792!Apr 16 2021, 2:25 PM
swh-public-ci added a comment.Apr 16 2021, 2:25 PM

Build was aborted

Patch application report for D5501 (id=19792)

Rebasing onto e86a39d9dc...

Current branch diff-target is up to date.
Changes applied before test
commit be8b495a5dedc63fc77bc7793fad587131b5f4d1
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/81/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/81/console

Hakimb updated this revision to Diff 19800.Apr 16 2021, 2:49 PM

I fixed the unit tests that didn't work because of the default argument value of max_edges (it was None instead of 0) in graph.py

swh-public-ci added a comment.Apr 16 2021, 2:51 PM

Build is green

Patch application report for D5501 (id=19800)

Could not rebase; Attempt merge onto 04a1656c10...

Merge made by the 'recursive' strategy.
 .../java/org/softwareheritage/graph/Entry.java     | 12 +++----
 .../java/org/softwareheritage/graph/Traversal.java | 37 ++++++++++++++++------
 swh/graph/backend.py                               | 18 ++++++++---
 swh/graph/client.py                                | 12 +++----
 swh/graph/graph.py                                 | 15 +++++----
 swh/graph/server/app.py                            | 31 ++++++++++++++----
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++
 7 files changed, 109 insertions(+), 39 deletions(-)
Changes applied before test
commit 9bf94be00fa0bf01eacd56edd6e04db21235a922
Merge: 04a1656 7d6ccb6
Author: Jenkins user <jenkins@localhost>
Date:   Fri Apr 16 12:49:47 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 7d6ccb63ede48d98aa2593043af59fa017de05c5
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Fri Apr 16 14:47:48 2021 +0200

    fix the default argument of max_edges on graph.py

commit be8b495a5dedc63fc77bc7793fad587131b5f4d1
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge

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

Harbormaster completed remote builds in B20813: Diff 19800.Apr 16 2021, 2:51 PM
Hakimb updated this revision to Diff 19815.Apr 16 2021, 5:38 PM

squashing of commits

swh-public-ci added a comment.Apr 16 2021, 5:39 PM

Build is green

Patch application report for D5501 (id=19815)

Rebasing onto 04a1656c10...

Current branch diff-target is up to date.
Changes applied before test
commit 9158c1cce7f370366dd6cb75933fba8f85a3592d
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py

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

Harbormaster completed remote builds in B20827: Diff 19815.Apr 16 2021, 5:40 PM
vlorentz requested changes to this revision.Apr 20 2021, 9:53 AM

In case you missed my message on IRC/Matrix:

The diff looks good now; but could you reword the commit message, as it does not need to include your old commit names. Just "Add an anti DoS limit for edges traversed as a query parameter" would be fine

This revision now requires changes to proceed.Apr 20 2021, 9:53 AM
Hakimb updated this revision to Diff 19872.Apr 20 2021, 4:29 PM

maxEdges is now a Traversal parameter

swh-public-ci added a comment.Apr 20 2021, 4:31 PM

Build is green

Patch application report for D5501 (id=19872)

Could not rebase; Attempt merge onto 04a1656c10...

Updating 04a1656..12856b9
Fast-forward
 .../java/org/softwareheritage/graph/Entry.java     | 12 ++++-----
 .../java/org/softwareheritage/graph/Traversal.java | 21 ++++++++++++++-
 swh/graph/backend.py                               | 18 +++++++++----
 swh/graph/client.py                                | 12 ++++-----
 swh/graph/graph.py                                 | 15 ++++++-----
 swh/graph/server/app.py                            | 31 +++++++++++++++++-----
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++++
 7 files changed, 102 insertions(+), 30 deletions(-)
Changes applied before test
commit 12856b9b16bb7e9f6ee9d2bfe0bf29f7be61880a
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 20 16:28:32 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter

commit 9158c1cce7f370366dd6cb75933fba8f85a3592d
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py

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

Harbormaster completed remote builds in B20883: Diff 19872.Apr 20 2021, 4:31 PM
Hakimb updated this revision to Diff 19873.Apr 20 2021, 4:48 PM

squash

swh-public-ci added a comment.Apr 20 2021, 4:50 PM

Build is green

Patch application report for D5501 (id=19873)

Rebasing onto 04a1656c10...

Current branch diff-target is up to date.
Changes applied before test
commit c534a2b8a0136015e6d5f51d2c9b68105f266586
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter
    
    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py
    
    Add an anti DoS limit for edges traversed as a query parameter

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

Harbormaster completed remote builds in B20884: Diff 19873.Apr 20 2021, 4:50 PM
Hakimb updated this revision to Diff 19902.Apr 21 2021, 3:29 PM
  • add the maxEdges parameter for leaves and neighbors endpoints
swh-public-ci added a comment.Apr 21 2021, 3:33 PM

Build has FAILED

Patch application report for D5501 (id=19902)

Rebasing onto 04a1656c10...

Current branch diff-target is up to date.
Changes applied before test
commit 23755dc5b3e10a228d6af8b04260fbd382b089bf
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 21 15:22:17 2021 +0200

    add the maxEdges parameter for leaves and neighbors endpoints

commit c534a2b8a0136015e6d5f51d2c9b68105f266586
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter
    
    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py
    
    Add an anti DoS limit for edges traversed as a query parameter

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/89/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/89/console

Harbormaster failed remote builds in B20908: Diff 19902!Apr 21 2021, 3:33 PM
Hakimb updated this revision to Diff 19946.Apr 22 2021, 8:51 PM
  • max_edges argument was missing
swh-public-ci added a comment.Apr 22 2021, 8:52 PM

Build is green

Patch application report for D5501 (id=19946)

Could not rebase; Attempt merge onto 04a1656c10...

Updating 04a1656..0e85d6b
Fast-forward
 .../java/org/softwareheritage/graph/Entry.java     | 20 +++++++-------
 .../java/org/softwareheritage/graph/Traversal.java | 31 +++++++++++++++++++++-
 swh/graph/backend.py                               | 14 +++++-----
 swh/graph/client.py                                | 22 ++++++++-------
 swh/graph/graph.py                                 | 17 +++++++-----
 swh/graph/server/app.py                            | 24 ++++++++++++++---
 swh/graph/tests/test_api_client.py                 | 23 ++++++++++++++++
 7 files changed, 115 insertions(+), 36 deletions(-)
Changes applied before test
commit 0e85d6b59b6284c29ef32bbed9d0fb9e4c61bd92
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Thu Apr 22 20:41:43 2021 +0200

    max_edges argument was missing

commit 23755dc5b3e10a228d6af8b04260fbd382b089bf
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Wed Apr 21 15:22:17 2021 +0200

    add the maxEdges parameter for leaves and neighbors endpoints

commit c534a2b8a0136015e6d5f51d2c9b68105f266586
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter
    
    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py
    
    Add an anti DoS limit for edges traversed as a query parameter

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

Harbormaster completed remote builds in B20950: Diff 19946.Apr 22 2021, 8:53 PM
Hakimb updated this revision to Diff 19948.Apr 22 2021, 8:57 PM

squash

swh-public-ci added a comment.Apr 22 2021, 8:59 PM

Build is green

Patch application report for D5501 (id=19948)

Rebasing onto 04a1656c10...

Current branch diff-target is up to date.
Changes applied before test
commit 369405ba4ba8fe4842bb52704f7aae58f77dba16
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter
    
    Add an anti DoS limit for edges traversed as a query parameter
    
    add an anti dos limit for edges traversed
    
    Combine 2 commit
    
    add an anti Ddos limit for edges traversed as a query parameter
    
    rebase + fix + merge
    
    fix the default argument of max_edges on graph.py
    
    Add an anti DoS limit for edges traversed as a query parameter
    
    add the maxEdges parameter for leaves and neighbors endpoints
    
    max_edges argument was missing

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

Harbormaster completed remote builds in B20951: Diff 19948.Apr 22 2021, 8:59 PM
Hakimb updated this revision to Diff 19969.Apr 23 2021, 2:51 PM

change commit name

swh-public-ci added a comment.Apr 23 2021, 2:53 PM

Build is green

Patch application report for D5501 (id=19969)

Rebasing onto 04a1656c10...

Current branch diff-target is up to date.
Changes applied before test
commit e8f052a34003bd9b32218b8a7caa2283953aa9df
Author: Hakim Baaloudj <hbaaloud@po461-pro.paris.inria.fr>
Date:   Tue Apr 13 17:30:18 2021 +0200

    Add an anti DoS limit for edges traversed as a query parameter.

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

Harbormaster completed remote builds in B20971: Diff 19969.Apr 23 2021, 2:53 PM
vlorentz accepted this revision.Apr 23 2021, 3:08 PM
This revision is now accepted and ready to land.Apr 23 2021, 3:08 PM
seirl accepted this revision.Apr 23 2021, 5:49 PM
Closed by commit rDGRPHe8f052a34003: Add an anti DoS limit for edges traversed as a query parameter. (authored by Hakimb). · Explain WhyApr 26 2021, 2:56 PM
This revision was automatically updated to reflect the committed changes.
Hakimb added a commit: rDGRPHe8f052a34003: Add an anti DoS limit for edges traversed as a query parameter..

Revision Contents
Changeset List

PathSize
java/
src/
main/
java/
org/
softwareheritage/
graph/
20 lines
31 lines
swh/
graph/
14 lines
22 lines
15 lines
server/
24 lines
tests/
23 lines

Diff 19902

View Options

java/src/main/java/org/softwareheritage/graph/Entry.java

Loading...
View Options

java/src/main/java/org/softwareheritage/graph/Traversal.java

Loading...
View Options

swh/graph/backend.py

Loading...
View Options

swh/graph/client.py

Loading...
View Options

swh/graph/graph.py

Loading...
View Options

swh/graph/server/app.py

Loading...
View Options

swh/graph/tests/test_api_client.py

Loading...
About · Developer information · Legal