On windows, check if the file is opened in binary mode#369
On windows, check if the file is opened in binary mode#369
Conversation
|
Why are all the builds suddenly failing? Seems to be related to pypa/setuptools#937. I'll poke around on it. Okay.. I'm not sure what's going on with Travis CI.. I'm going to wait a day and see if they figure it out/clear it up. |
dpkt/pcap.py
Outdated
| """ | ||
|
|
||
| def __init__(self, fileobj, snaplen=1500, linktype=DLT_EN10MB, nano=False): | ||
| if fileobj.mode != 'wb': |
There was a problem hiding this comment.
add a check for Windows, same as below?
There was a problem hiding this comment.
Oops! yes, thanks for catching
dpkt/pcap.py
Outdated
| self.name = getattr(fileobj, 'name', '<%s>' % fileobj.__class__.__name__) | ||
| if sys.platform.startswith('win'): | ||
| if fileobj.mode != 'rb': | ||
| raise ValueError('PCAP file (%s) not opened in binary mode (rb)'%self.name) |
There was a problem hiding this comment.
pep8
... (rb)' % self.name)
^ ^ spaces
dpkt/pcap.py
Outdated
| def __init__(self, fileobj): | ||
| self.name = getattr(fileobj, 'name', '<%s>' % fileobj.__class__.__name__) | ||
| if sys.platform.startswith('win'): | ||
| if fileobj.mode != 'rb': |
There was a problem hiding this comment.
both mode checks feel a bit too restrictive (but make complete sense at the same time..). I personally would go with smth like if 'b' not in fileobj.mode
There was a problem hiding this comment.
Just noticed that i did not account for append modes, your suggestion takes care of that too.
|
Okay the tests on TravisCI are now properly running again... there are two failed tests... and then a test for the deprecated decorator (I don't totally understand why that is failing). |
|
going to look at this some more later today. I have a bunch of more updates for pcap.py anyway |
While testing the PR some more, I noticed that python3 on Linux did not open in binary mode when we do no pass 'b'. Therefore it seemed appropriate to expand this check to all OS, and not just windows.
dpkt/pcap.py
Outdated
|
|
||
| def __init__(self, fileobj): | ||
| self.name = getattr(fileobj, 'name', '<%s>' % fileobj.__class__.__name__) | ||
| if sys.platform.startswith('win'): |
There was a problem hiding this comment.
So do we want to just check 'rb' mode for all platforms?
|
Shrug... somehow the builds are failing... will have to circle back to this and see if I can figure out the issues... |
4 participants
Often times, windows users open the files with the wrong mode. Unfortunately, the file mode property is read-only, so we can not fix it on the fly. Raise an exception instead.