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

Fix reading from buffer not at start of file and using autodetection #2419

Merged
merged 3 commits into from Sep 18, 2019

Conversation

@megies
Copy link
Member

commented Jun 24, 2019

I am pretty sure we have a bug in our reading routines when

  • auto fileformat detection is used
  • a binary buffer is passed
  • that buffer is not at start of file

To reproduce, add some random bytes into a buffer and fill the rest with a valid waveform files data:

from obspy import read
filename = 'LMOW.BHE.SAC'  # file is in io/sac/tests/data
import io
bio = io.BytesIO()
bio.write(b'123456')
data = open(filename, 'rb').read()
bio.write(data)
bio.seek(6)
st = read(bio)

The last line will call _is_mseed() first, and then goes over to _is_sac() and should eventually read it as a sac file starting at the initial offset position of 6 bytes.

This might be a real edge case, especially since some is_xxx() check routines might not be clean about keeping file handles open (did not check all of them..) but still should be fixed eventually.

The fix should be simple, but I found more code that breaks when not at position 0 in buffer (in sac file check) and there might be many more locations with problems of that kind, but still better to fix at least some of them. We probably need a _generic_is_file_format routine that consistently handles open file handles etc like was done in #2326 not sure..

diff --git a/obspy/core/util/base.py b/obspy/core/util/base.py
index 9c81efe17..0a9ffbcea 100644
--- a/obspy/core/util/base.py
+++ b/obspy/core/util/base.py
@@ -433,7 +434,7 @@ def _read_from_plugin(plugin_type, filename, format=None, **kwargs):
             # check format
             is_format = is_format(filename)
             if position is not None:
-                filename.seek(0, 0)
+                filename.seek(position, 0)
             if is_format:
                 break
         else:

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just remove the space in the following string after the + sign: "+ DOCS"
  • If any network modules should be tested for the PR, add them as a comma separated list
    (e.g. clients.fdsn,clients.arclink) after the colon in the following magic string: "+TESTS:"
    (you can also add "ALL" to just simply run all tests across all modules)
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
  • Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
@megies megies added bug .core labels Jun 24, 2019
@megies megies added this to the 1.2.0 milestone Sep 17, 2019
@megies megies added this to Waiting for Review in Release 1.2.0 Sep 17, 2019
megies added 3 commits Jun 24, 2019
The seek was simply going to start of file, not to the saved position
relative to start of file
start of
file
@megies megies force-pushed the fix_read_autodetect_buffer_position branch from 1fea8c4 to c032080 Sep 17, 2019
@megies

This comment has been minimized.

Copy link
Member Author

commented Sep 17, 2019

Rebased + force-pushed

@megies megies merged commit e69ce0c into master Sep 18, 2019
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
@megies megies deleted the fix_read_autodetect_buffer_position branch Sep 18, 2019
@megies megies moved this from Waiting for Review to Done in Release 1.2.0 Sep 18, 2019
megies added a commit that referenced this pull request Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
1 participant
You can’t perform that action at this time.