-
Notifications
You must be signed in to change notification settings - Fork 38
Fix string length computation for issue_command function #33
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,13 @@ | |
| SERVER_EXEC = os.path.abspath(os.path.join(os.path.dirname(__file__), | ||
| 'webkit_server')) | ||
|
|
||
| if sys.version_info[0] == 2: | ||
| PY2 = True | ||
| PY3 = False | ||
| elif sys.version_info[0] == 3: | ||
| PY2 = False | ||
| PY3 = True | ||
|
|
||
|
|
||
| class SelectionMixin(object): | ||
| """ Implements a generic XPath selection for a class providing | ||
|
|
@@ -513,12 +520,47 @@ def issue_command(self, cmd, *args): | |
| self._writeline(cmd) | ||
| self._writeline(str(len(args))) | ||
| for arg in args: | ||
| arg = str(arg) | ||
| self._writeline(str(len(arg))) | ||
| self._sock.sendall(arg.encode("utf-8")) | ||
| self.send_arg(arg) | ||
|
|
||
| return self._read_response() | ||
|
|
||
| def send_arg(self, arg): | ||
| """ Send each arg for args in issue_command """ | ||
| arg = self.prepare_for_socket_sendall(arg) | ||
| self._writeline(str(len(arg))) | ||
| self._sock.sendall(arg) | ||
|
|
||
| def prepare_for_socket_sendall(self, arg): | ||
| """ Deal with the unicode and bytes problem in Python 2 and Python 3 for socket.sendall """ | ||
|
|
||
| if PY2: | ||
| if type(arg) != unicode: | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if this raise an NameError in Python 3 since there's no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this would be a problem in other languages such as C++, where identifiers need to be defined at compilation time even if they are never used at runtime, this should not be an issue for Python, since here definitions of identifiers are only checked when encountered, and not during the syntax check. In Python, it is also common to import packages only when they are required, and therefore Therefore, I do not expect to find any problems with this line of code. In addition, I have successfully tested the suggested changes while observing no error messages or warnings. |
||
| try: | ||
| arg = str(arg) | ||
| except: | ||
| pass | ||
|
|
||
| # socket.sendall in Python 2 accepts str (bytes) | ||
| if type(arg) not in (str, unicode): | ||
| raise TypeError("type({}) should be str (bytes) or unicode, not {}".format(arg, type(arg))) | ||
| elif type(arg) == unicode: | ||
| arg = arg.encode("utf-8") | ||
|
|
||
| if PY3: | ||
| if type(arg) != bytes: | ||
| try: | ||
| arg = str(arg) | ||
| except: | ||
| pass | ||
|
|
||
| # socket.sendall in Python 3 accepts bytes | ||
| if type(arg) not in (str, bytes): | ||
| raise TypeError("type({}) should be str or bytes, not {}".format(arg, type(arg))) | ||
| elif type(arg) == str: | ||
| arg = arg.encode("utf-8") | ||
|
|
||
| return arg | ||
|
|
||
| def _read_response(self): | ||
| """ Reads a complete response packet from the server """ | ||
| result = self.buf.read_line().decode("utf-8") | ||
|
|
@@ -538,4 +580,5 @@ def _read_message(self): | |
|
|
||
| def _writeline(self, line): | ||
| """ Writes a line to the underlying socket. """ | ||
| self._sock.sendall(line.encode("utf-8") + b"\n") | ||
| line = self.prepare_for_socket_sendall(line) | ||
| self._sock.sendall(line + b"\n") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be using an indentation of 2 spaces rather than 4 consistently throughout the file. I suggest correcting the indentation of line 523.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've fixed it in this commit 62ad2e9.