Skip to content

Gmail _extract_body: decode failures misreported as absence, and placeholder strings can clobber drafts #203

Description

@peteski22

Summary

_extract_body in src/apron_tools/providers/google/gmail/tools.py returns human-readable sentinel strings on failure. Those sentinels are (a) inconsistent between the single-part and multipart paths, and (b) fed into write paths where they cause silent data loss. Pre-existing and module-wide; surfaced while reviewing #200 (base64url padding) and deliberately kept out of that PR to keep it focused.

1. Decode failure misreported as absence

The single-part path returns (Could not decode email body) on a decode failure, but the multipart text/plain/text/html branches use contextlib.suppress and fall through to (No email body found). For the same underlying condition — a body that is present but cannot be decoded — the two paths give the caller contradictory signals (decode failure vs absence).

2. Recursion guard compares a magic string

elif part.get("parts"):
    nested = _extract_body(part)
    if nested and nested != "(No email body found)":
        return nested

Two problems:

  • The guard compares against only one sentinel, so a nested (Could not decode email body) is treated as a successful extraction and short-circuits the loop — even when a sibling part carries a decodable body.
  • return nested fires immediately, discarding a plain_text already collected from an earlier sibling, which violates the docstring's stated "preferring text/plain over text/html" ordering when a text/plain sibling precedes a multipart sibling.

3. Placeholder strings can clobber a draft (data loss)

gmail_edit_draft uses:

final_body = params.body if params.body is not None else _extract_body(payload)

When the caller does not supply a new body, a decode failure or unrecognized payload shape overwrites the existing draft body with the literal text (Could not decode email body) / (No email body found) and reports success — unrecoverable data loss presented as a success.

Suggested direction

Make the recursive extractor return str | None (no sentinel inside the recursion), collecting nested results into the same plain/html preference resolution rather than short-circuiting. Render the user-facing sentinel exactly once at the gmail_read_email display boundary, and have gmail_edit_draft return success=False with a real error when the existing body cannot be recovered rather than clobbering it. This removes the magic-string comparison (items 1 and 2) and fixes the data-loss path (item 3) together.

Scope note

Deferred from #200. Related: #169 (module hardening — base64url padding + unguarded resp.json()).

A related non-validating-decode concern (lenient urlsafe_b64decode silently discarding out-of-alphabet characters, risking truncated/corrupted bytes) was originally noted here but has since been fixed in #200 (commit 20ec75d, strict base64url validation via base64.b64decode(..., altchars=b"-_", validate=True)), so it is out of scope for this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    googleProvider: googleproviderIssue or PR scoped to a specific provider

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions