Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4528 closed defect (fixed)

read_to_buf_tls(): Inconsistency in code

Reported by: troll_un Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

flush_buf_tls() peek for any pending OpenSSL errors before write operation.

read_to_buf_tls() do not pending errors before read operation. No check_no_tls_errors()

Child Tickets

Change History (7)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final

comment:2 Changed 8 years ago by hellais

Status: newneeds_review

If I understand correctly what you mean in this bug you are saying that while we check for check_no_tls_errors() inside of flush_buf_tls() we don't do the same in read_to_buf_tls().

I looked at it and you appear to be correct. This is fixed inside of my master branch: https://gitweb.torproject.org/user/art/tor.git.

comment:3 Changed 8 years ago by nickm

PLEASE don't use your master branch as a catch-all for everything you might want to fix. Please make a topic branch instead.

There are 21 commits in your master branch that aren't in the upstream master. I'm guessing 7e4c36849bc9c038b2a6 is the one to look at. That one seems ok. I'll test it a little then cherry-pick it.

comment:4 in reply to:  3 ; Changed 8 years ago by hellais

Replying to nickm:

PLEASE don't use your master branch as a catch-all for everything you might want to fix. Please make a topic branch instead.

Yeah I'm sorry about that, I realized it after I had already done it.

I have created a separate branch for this issue now:
https://gitweb.torproject.org/user/art/tor.git/shortlog/refs/heads/bug4744

comment:5 in reply to:  4 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Replying to hellais:

Replying to nickm:

PLEASE don't use your master branch as a catch-all for everything you might want to fix. Please make a topic branch instead.

Yeah I'm sorry about that, I realized it after I had already done it.

I have created a separate branch for this issue now:
https://gitweb.torproject.org/user/art/tor.git/shortlog/refs/heads/bug4744

But this is bug 4528. Bug4744 appears to be something else.

I just made a new branch "bug4528" in my public repo, cherry-picked the commit, added a changes file, made the commit message say more about what the patch did, and merged it all as e0e4b8475708729a5746. Thanks!

comment:6 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:7 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.