Opened 9 years ago

Closed 9 years ago

#462 closed defect (fixed)

b10-auth includes trailing garbage in responses

Reported by: jinmei Owned by: jinmei
Priority: very high Milestone: y2 12 month milestone
Component: b10-auth Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is a side effect of the recent heavy change to ASIO link.

Subtickets

Change History (6)

comment:1 Changed 9 years ago by jinmei

  • Owner changed from jinmei to UnAssigned
  • Status changed from new to reviewing

branches/trac462 is ready for review.

This is a critical bug and the fix must be merged before the next release.

I also suspect the DNSAnswer framework is too much for b10-auth, and its performance overhead won't be acceptable once we start focusing on response performance. We'll then simply drop using it, but for now we should fix it with a minimal change.

The proposed ChangeLog? entry is as follows:

  141.?	[bug]		jinmei
	b10-auth: Fixed a bug that the authoritative server includes
	trailing garbage data in responses.  This is a regression due to
	change #135. (Trac #462, svn rTBD)

comment:2 Changed 9 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:3 follow-up: Changed 9 years ago by vorner

  • Owner changed from vorner to jinmei

The tests looks fine and it responds with reasonable size of data, so I think it can be merged.

Just out of curiosity, the code inside the operator () was duplicated somewhere else as well? Because it just disappeared, and seems not to be missed. Is there other part that does the same?

comment:4 Changed 9 years ago by jreed

This looks to be the same bug as #424. I will close #424 if this fixes it.

comment:5 in reply to: ↑ 3 Changed 9 years ago by jinmei

Replying to vorner:

The tests looks fine and it responds with reasonable size of data, so I think it can be merged.

Okay, thanks for review.

Just out of curiosity, the code inside the operator () was duplicated somewhere else as well? Because it just disappeared, and seems not to be missed. Is there other part that does the same?

b10-auth constructs the response message (which is what operator() did (in an incomplete way)) as part of query processing. For example, AuthSrvImpl::processNormalQuery() does this here:

    MessageRenderer renderer(*buffer);
    const bool udp_buffer =
        (io_message.getSocket().getProtocol() == IPPROTO_UDP);
    renderer.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
    message->toWire(renderer);

comment:6 Changed 9 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed

merged to trunk, closing ticket.

Note: See TracTickets for help on using tickets.