Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket33072 042 01 #1858

Open
wants to merge 2 commits into
base: maint-0.4.2
Choose a base branch
from

Conversation

dgoulet-tor
Copy link
Contributor

No description provided.

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 <[email protected]>
@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

@@ -56,7 +56,7 @@ problem dependency-violation /src/core/crypto/onion_crypto.c 5
problem dependency-violation /src/core/crypto/onion_fast.c 1
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