GetTotalBytesSent() can't parse value, if it's negative

[karnauskas]

Sent:

POST /upnp/control/WANCommonIFC1 HTTP/1.1
Host: 192.168.1.1:4562
User-Agent: Go-http-client/1.1
Content-Length: 302
CONTENT-TYPE: text/xml; charset="utf-8"
SOAPACTION: "urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1#GetTotalBytesSent"
Accept-Encoding: gzip

<?xml version="1.0" encoding="UTF-8"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><s:Body><u:GetTotalBytesSent xmlns:u="urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1"></u:GetTotalBytesSent></s:Body></s:Envelope>

Received:

HTTP/1.1 200 OK
CONTENT-LENGTH:334
CONTENT-TYPE:text/xml
DATE: Fri, 21 Jun 2019 21:00:19 GMT
EXT:
SERVER:Linux/2.6.36 UPnP/1.0 myigd/1.0

<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><s:Body>
<u:GetTotalBytesSentResponse xmlns:u="urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1">
<NewTotalBytesSent>-1363540943</NewTotalBytesSent>
</u:GetTotalBytesSentResponse>
</s:Body></s:Envelope>.

AFAIK this device is not following standards. However, miniupnpc client is not failing to parse this response.

[huin]

Hm, that's fiddly. But not the first time that a device doesn't match the standard in this place.

There's an existing "hack" here: https://github.com/huin/goupnp/blob/master/cmd/goupnpdcpgen/metadata.go#L57

The UPNP standard states "ui4" (aka uint32 in Go), which this hack promotes to "ui8" (uint64 as you've probably seen). This was contributed by someone else using this library who ran into a device that exceeded a value of 2^31-1.

Two options spring to mind:

1. We could try changing this hack from "ui8" to "int" (yeah, gotta love the consistency: https://github.com/huin/goupnp/blob/master/cmd/goupnpdcpgen/typemap.go#L18). The thing that bothers me is that it could cause incompatibilities with other people using this library -- i.e their existing code reads this value into a variable/field of type uint64, which would now no longer compile.

2. Another alternative might be to try and introduce another hack that creates another method that has a different name, calls the same SOAP method, but returns int64 instead. A bit more complex and ugly, but keeps compatibility.

Thoughts?

[karnauskas]

I would suggest to follow standards and make less hacks. AFAIK it works without any problem with miniupnpd, tho not with some weird implementations. Even, if goupnp could parse that value, it's going like sinusoid - difficult to track existing state. I would rather allow to let weird implementation router to deprecate.

[huin]

Just revisiting this years later. A workaround in specific cases where this happens could be to fork a copy of the implementation of the existing function, and use that instead of changing this package.