Skip to content

fix: GzipParser passes through non-gzipped data instead of raising BadGzipFile#1072

Draft
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783966700-gzip-parser-passthrough
Draft

fix: GzipParser passes through non-gzipped data instead of raising BadGzipFile#1072
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783966700-gzip-parser-passthrough

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

GzipParser.parse documented that "if the data is not gzipped, pass the data to the inner parser as is", but never implemented it — it unconditionally wrapped the stream in gzip.GzipFile, so any layer that isn't actually gzip crashes with BadGzipFile: Not a gzipped file (...).

This bites declarative connectors that stack decoders, because create_gzip_decoder builds a header-selected decoder whose fallback strips exactly one gzip layer:

# GzipDecoder -> GzipDecoder -> CsvDecoder  becomes:
#   header match -> GzipParser(GzipParser(CsvParser))   # 2 gzip layers assumed
#   fallback     -> GzipParser(CsvParser)               # exactly 1 gzip layer assumed

When a response intermittently arrives with fewer gzip layers than configured (or as an unexpected format, e.g. a zip archive with magic b'PK'), the rigid parser raises BadGzipFile and the stream fails. This surfaced as an intermittent BadGzipFile: Not a gzipped file (b'PK') on a custom App Store Connect connector's subscription_event_report stream.

Fix: peek the leading bytes (via io.BufferedReader, which does not consume the stream and works uniformly over urllib3 raw responses, GzipFile, and BytesIO) and only gzip-decompress when the gzip magic number is present; otherwise hand the stream straight to the inner parser.

buffered = data if isinstance(data, io.BufferedReader) else io.BufferedReader(data)
if buffered.peek(2)[:2] == b"\x1f\x8b":
    with gzip.GzipFile(fileobj=buffered, mode="rb") as gz:
        yield from self.inner_parser.parse(gz)
else:
    yield from self.inner_parser.parse(buffered)

This makes nested/fallback gzip decoding tolerant of 0, 1, or N gzip layers and is fully backward-compatible when data is gzipped. Note: a genuinely zip-compressed (b'PK') payload still needs a ZipfileDecoder for that layer — this change stops the hard crash but does not unzip.

Tests

  • New unit tests in test_composite_decoder.py: single GzipParser passthrough on uncompressed data; nested GzipParser(GzipParser(CsvParser)) handling both one and two gzip layers; header-based fallback to a GzipParser on a non-gzipped response.
  • Existing decoder + streaming memory-usage tests still pass.

Link to Devin session: https://app.devin.ai/sessions/862184b2ca174e4483b2cc1b5264e978

…dGzipFile

Co-Authored-By: lucas.leadbetter@airbyte.io <5595530+lleadbet@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1783966700-gzip-parser-passthrough#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1783966700-gzip-parser-passthrough

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 132 tests  +5   4 120 ✅ +5   7m 47s ⏱️ +6s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 73f1521. ± Comparison against base commit dd9558c.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 135 tests  +5   4 123 ✅ +5   11m 42s ⏱️ + 4m 27s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 73f1521. ± Comparison against base commit dd9558c.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants