Skip to content

Conversation

dgoulet-tor
Copy link
Contributor

No description provided.

Fixes #33072

Signed-off-by: David Goulet <dgoulet@torproject.org>
This also adds the support for the option meaning, if set, every uncompressed
directory requests will be served by the dirauth with a 503 error code.

Signed-off-by: David Goulet <dgoulet@torproject.org>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8670

  • 8 of 9 (88.89%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 62.351%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/dircache/dircache.c 4 5 80.0%
Totals Coverage Status
Change from base Build 8562: 0.001%
Covered Lines: 48566
Relevant Lines: 77891

💛 - Coveralls

problem dependency-violation /src/core/crypto/onion_tap.c 3
problem dependency-violation /src/core/crypto/relay_crypto.c 9
problem file-size /src/core/mainloop/connection.c 5569
problem file-size /src/core/mainloop/connection.c 5694
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, you shouldn't need to fix practracker on 0.4.2 or 0.4.3.

I have removed check-best-practices from check-local on all maint and release branches. So practracker only runs by default on master.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the 0.4.2 and master code got out of sync with 0.4.3.
And the function comments need to be updated in every branch.

Comment on lines +3227 to +3230
if (c_method != NO_METHOD) {
/* Always answer compressed request. */
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is missing in 0.4.3, but present in 0.4.2 and master.

Did you forget to delete it in 0.4.2 and master?

Comment on lines +3193 to +3197
* If we are a directory authority, false is returned (indicating that we
* should answer the request) if one of these conditions is met:
* - Connection is from a known relay (address is looked up).
* - AuthDirRejectRequestsUnderLoad is set to 0.
* - Compression is being used for the request (looking at c_method).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't match what the function does in 0.4.3.

@@ -0,0 +1,4 @@
o Minor bugfix (directory authority):
- Always allow compressed directory requests and introduce option
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the changes file is out of sync as well.

We let AuthDirRejectRequestsUnderLoad and the bandwidth buckets decide about compressed requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants