Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#30868 closed defect (not a bug)

Modify client rendezvous library to remove hard-coded responses

Reported by: cohosh Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Circumvention/Snowflake Version: Tor: unspecified
Severity: Normal Keywords: ex-sponsor-19, easy
Cc: dcf, arlolra, cohosh, phw Actual Points:
Parent ID: #29259 Points:
Reviewer: Sponsor: Sponsor28

Description

Client tests rely of client/lib/rendezvous.go rely on specific HTTP response bodies which are prone to change and unnecessary

Child Tickets

Change History (3)

comment:1 Changed 2 months ago by cohosh

Resolution: not a bug
Status: newclosed

To be more specific, I was thinking of

        BrokerError503        string = "No snowflake proxies currently available."
        BrokerError400        string = "You sent an invalid offer in the request."
        BrokerErrorUnexpected string = "Unexpected error, no answer."

but on closer look, I don't see any problems with this. Closing.

comment:2 Changed 2 months ago by dcf

I thought the problem was just in some tests. I am not sure, but it looks like this test is checking the for the literal string "No snowflake proxies currently available." rather than a status code of 503.

		Convey("BrokerChannel.Negotiate fails with 503", func() {
			b := NewBrokerChannel("test.broker", "",
				&MockTransport{http.StatusServiceUnavailable, []byte("\n")})
			answer, err := b.Negotiate(fakeOffer)
			So(err, ShouldNotBeNil)
			So(answer, ShouldBeNil)
			So(err.Error(), ShouldResemble, BrokerError503)
		})

comment:3 in reply to:  2 Changed 2 months ago by dcf

Replying to dcf:

it looks like this test is checking the for the literal string "No snowflake proxies currently available." rather than a status code of 503.

No, I was mistaken. The test is just checking the text of the error message, which is okay. The broker returns an empty body for status 503, and here is where the status codes get translated into error strings.

	case http.StatusServiceUnavailable:
		return nil, errors.New(BrokerError503)
	case http.StatusBadRequest:
		return nil, errors.New(BrokerError400)
Note: See TracTickets for help on using tickets.