Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve RCON message body decoding #14

Open
Holiverh opened this issue Nov 14, 2014 · 6 comments
Open

Improve RCON message body decoding #14

Holiverh opened this issue Nov 14, 2014 · 6 comments
Assignees

Comments

@Holiverh
Copy link
Member

#13 highlighted a case where the body of an RCON message (a response to a status command to be specific) may not be valid ASCII.

This is contrary to the Valve wiki page for RCON which seems to imply that RCON message bodies should be null terminated ASCII encoded strings. Evidently this isn't always the case. As I was unable to recreate the issue for my self I can't really dig into it much further to know what encoding srcds is actually using. Perhaps its some kind of "extended ASCII", e.g. ISO 8859-1? Maybe it's UTF-8. Who knows, but I don't think we should simply guess.

2d56e64 was added as a workaround for #13 but this is not ideal as it simply ignores anything it can't decode.

Looking at the two other Python RCON implementations (SourceLib and pysrcds) neither of them seem to even attempt to decode the message body, leaving it as a bytestring. Whilst no confident assertions can be made about the encoding of RCON messages there is merit to this behavior IMO.

Ultimately:

  • UnicodeDecodeError should never be propagated from Message.decode.
  • ASCII should be assumed until the actual encoding can be determined.
  • Message should expose the message body as a bytestring along side the Unicode body so that if there is any data loss from ignoring decode errors the user at least has a chance of salvaging the real message body by handling the decoding themselves or by just using the bytestring bodies exclusively.

🎱

@Holiverh
Copy link
Member Author

These changes will more or less be incorporated into a rewrite of the RCON module.

@Holiverh Holiverh added this to the 1.0.0 milestone Nov 30, 2015
@Holiverh Holiverh self-assigned this Nov 30, 2015
@Holiverh Holiverh removed this from the 1.0.0 milestone Sep 23, 2017
@brandonsturgeon
Copy link

brandonsturgeon commented Mar 21, 2018

Any update on this one? I tried changing rcon.py from ascii to utf-8 and am presented with this error:

Unexpected message <RCONMessage 0 AUTH_RESPONSE 0B>
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
UnicodeEncodeError: 'ascii' codec can't encode characters in position 994-996: ordinal not in range(128)

Quite unhelpful. I'm not sure why ascii is even still being used.

I've tried pretty much everything mentioned in the docs. I can't run valve.rcon.RCON.execute() without this error - I don't even get a chance to encode/decode with my desired format.

@dieselburner
Copy link

dieselburner commented Jan 7, 2019

I've hit this issue yesterday. I have my own web interface for the server I'm running, which is dependent on this package. It was stable as hell until someone joined a server with a weird chars in his name. That's when simpe status request was crashing my app:

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 2309, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 2295, in wsgi_app
    response = self.handle_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1741, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 2292, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1815, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1718, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1799, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "./application.py", line 45, in decorator
    return f(*args, **kwargs)
  File "./application.py", line 73, in index
    data = rcon('status')
  File "/usr/local/lib/python2.7/dist-packages/valve/rcon.py", line 346, in __call__
    raise RCONMessageError("Couldn't decode response: {}".format(exc))
valve.rcon.RCONMessageError: Couldn't decode response: 'utf8' codec can't decode byte 0xd0 in position 359: invalid continuation byte

After looking into detalis a bit further it does not matter what encoding you are using - you might hit this issue in any case. IMO, quick (right?) solution would be to simply drop those chars that can't be encoded:

text.encode(encoding=self.ENCODING, errors='ignore')

I'll create a PR, and then it's up to you to decide what would be the correct way to do this.

@dieselburner
Copy link

Here you go. Feel free to update a commit message if needed.

@ip2k
Copy link

ip2k commented Sep 23, 2019

For others who may find this in the future, this is what I found a solution in:

First, read https://python-valve.readthedocs.io/en/latest/rcon.html#unicode-and-string-encoding

with valve.rcon.RCON(server_address, password) as rcon:
    rcon_resp = rcon('status')
rcon_text = rcon_resp.body.decode('utf-8', 'ignore')

This works for me on a Ubuntu 18 LTS server running this under Python 3.6 against SRCDS running on the same instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants