Page MenuHomeSoftware Heritage

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

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

Details

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 Graph service
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 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.

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)

This revision now requires changes to proceed.Tue, Apr 13, 6:37 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.Wed, Apr 14, 3:58 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

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

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

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

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?

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

@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")

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

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

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

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.

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.

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.Tue, Apr 20, 9:53 AM

maxEdges is now a Traversal parameter

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.

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.

  • add the maxEdges parameter for leaves and neighbors endpoints

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

  • max_edges argument was missing

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.

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.

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.

This revision is now accepted and ready to land.Fri, Apr 23, 3:08 PM