500 `Invalid WRP content type: */*` for an invalid accept header

[denopink]

500 `Invalid WRP content type: */*` for an invalid accept header

[kristinapathak]

Given where the error is originating has no concept of what header it is trying to parse, I don't think it's worth it to change the error string. This should be a 400 though.

[ilawjr]

@denopink did you still need my help?

[denopink]

@renaz6 looks like we still get a 500 with the current talaria repo using github.com/xmidt-org/wrp-go/v3 v3.1.1 locally and with the latest image docker run -p 6200-6204:6200-6204 xmidt/talaria:latest (re-pulled the image to make sure)

[denopink]

@ilawjr hey Ivan, can you help me test/verify this issue? 🧑‍🔬
I'm seeing the 500 locally atm.

[renaz6]

This issue was fixed in WRP-Go v3.1.1 and Talaria v0.6.4. @denopink could you verify the Talaria version you are using locally?

[denopink]

@renaz6 looks like it should have been the latest. Let me redeploy locally and confirm.

[denopink]

tl;rd

Update
https://github.com/xmidt-org/wrp-go/blob/4b275411c7aa52e1a537cc130f3b0f37e526c978/wrphttp/requestResponse.go#L140-L143

func NewEntityResponseWriter(defaultFormat wrp.Format) ResponseWriterFunc {
	return func(httpResponse http.ResponseWriter, wrpRequest *Request) (ResponseWriter, error) {
		format, err := DetermineFormat(defaultFormat, wrpRequest.Original.Header, "Accept")
		if err != nil {
			return nil, err
		}
		...
	}
}

to

func NewEntityResponseWriter(defaultFormat wrp.Format) ResponseWriterFunc {
	return func(httpResponse http.ResponseWriter, wrpRequest *Request) (ResponseWriter, error) {
		format, err := DetermineFormat(defaultFormat, wrpRequest.Original.Header, "Accept")
		if err != nil {
			wrappedErr := httpError{
				err:  err,
				code: http.StatusBadRequest,
			}
			return nil, wrappedErr
		}
		...
	}
}

Explanation

Looked into it and it looks like we're not handling a bad format error from DetermineFormat at
https://github.com/xmidt-org/wrp-go/blob/4b275411c7aa52e1a537cc130f3b0f37e526c978/wrphttp/requestResponse.go#L140-L143

func NewEntityResponseWriter(defaultFormat wrp.Format) ResponseWriterFunc {
	return func(httpResponse http.ResponseWriter, wrpRequest *Request) (ResponseWriter, error) {
		format, err := DetermineFormat(defaultFormat, wrpRequest.Original.Header, "Accept")
		if err != nil {
			return nil, err
		}
		...
	}
}

Then NewEntityResponseWriters DetermineFormat error gets handled as a 500 at
https://github.com/xmidt-org/wrp-go/blob/4b275411c7aa52e1a537cc130f3b0f37e526c978/wrphttp/handler.go#L134-L138

func (wh *wrpHandler) ServeHTTP(httpResponse http.ResponseWriter, httpRequest *http.Request) {
	...
	wrpResponse, err := wh.newResponseWriter(httpResponse, wrpRequest)
	if err != nil {
		wh.errorEncoder(wrpRequest.Context(), err, httpResponse)
		return
	}
	...
}

Tested locally with a quick patch and it looks goods:

curl -v --location --request POST 'localhost:6200/api/v3/device/send' \
--header 'Content-Type: application/json' \
--header 'Authorization: Basic dXNlcjpwYXNz' \
--data-raw '{
"msg_type":3,
"content_type":"application/json",
"source":"dns:me",
"dest":"mac:112233445566",
"transaction_uuid":"1234567890",
"payload":"eyJjb21tYW5kIjoiR0VUIiwibmFtZXMiOlsiU29tZXRoaW5nIl19",
"partner_ids":["comcast"]
}'

# Output:

Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:6200...
* Connected to localhost (127.0.0.1) port 6200 (#0)
> POST /api/v3/device/send HTTP/1.1
> Host: localhost:6200
> User-Agent: curl/7.79.1
> Accept: */*
> Content-Type: application/json
> Authorization: Basic dXNlcjpwYXNz
> Content-Length: 223
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< Content-Length: 29
< Content-Type: text/plain; charset=utf-8
< X-Talaria-Build: 0.1.4
< X-Talaria-Flavor: mint
< X-Talaria-Region: east
< X-Talaria-Server: talaria
< X-Talaria-Start-Time: 26 Apr 22 15:51 UTC
< Date: Tue, 26 Apr 2022 16:03:41 GMT
< 
* Connection #0 to host localhost left intact
Invalid WRP content type: */*%  
#