Skip to content

Case-insensitive IMAP response parsing (RFC 3501) - #116

Open
mpscholten wants to merge 2 commits into
qnikst:masterfrom
mpscholten:codex/imap-case-insensitive
Open

Case-insensitive IMAP response parsing (RFC 3501)#116
mpscholten wants to merge 2 commits into
qnikst:masterfrom
mpscholten:codex/imap-case-insensitive

Conversation

@mpscholten

Copy link
Copy Markdown
Contributor

Stacked on #115 (UIDPLUS). This branch is based on the UIDPLUS branch, so until #115 merges the diff here also shows those commits — the change that belongs to this PR is the single commit "Make IMAP response parsing case-insensitive (RFC 3501)". Please review/merge #115 first; I'll rebase this onto master afterwards so the diff narrows to just the parser change.

What

RFC 3501 keywords, response codes, flag names and status attributes are case-insensitive, but the response parser matched them with case-sensitive string. A server replying with non-canonical casing — a001 ok, * search …, [uidvalidity …], \seen — caused a hard parse error. Several real-world servers (Exchange/Office365 among them) don't use the exact canonical uppercase.

This adds stringCI / charCI and applies them throughout the parser.

While restructuring, also fixed

  • factor the repeated "parse untagged lines then the tagged line" pattern into pWithTaggedOrFatal
  • split pDone into pRespCode / pRespText / pStatusCode
  • unify ad-hoc string parsing into pQuotedString / pLiteralString / pAString / pMailboxName
  • surface an untagged * BYE as a fatal response (BAD) instead of failing to parse
  • accept an empty * SEARCH reply (zero matches)
  • accept a NIL hierarchy separator in LIST/LSUB
  • stop atomChar from running past CR/LF

Tests

New caseInsensitiveTest group: lowercase tagged/bracketed status codes, lowercase flags, untagged BYE, empty SEARCH, lowercase SEARCH keyword, NIL LIST separator. cabal test passes (GHC 9.8.4).

The suite has one pre-existing failure on master unrelated to this PR (append preserves raw crlf message bytes — expects unquoted APPEND INBOX); left untouched.

🤖 Generated with Claude Code

Adds UIDPLUS extension support so callers can recover the UIDs the
server assigns on APPEND/COPY and target expunges by UID:

  - appendFullUID: like appendFull, returns the APPENDUID response code
  - copyUID / copyUIDs / copyUIDR: UID COPY returning the COPYUID code
  - uidExpunge / uidExpungeR: UID EXPUNGE over a UID set or range

New types AppendUID and CopyUID, the UIDSet alias, and the
APPENDUID/COPYUID/UIDNOTSTICKY status codes with their parsers.
sendCommandWithResponse exposes the tagged ServerResponse so the
response codes can be read. The existing appendFull/copyFull keep their
old signatures by discarding the UID result.

Covered by new parser cases in baseTest and a dedicated imapUIDPlusTest
group exercising the API against scripted server responses.
RFC 3501 keywords, response codes, flag names and status attributes are
case-insensitive, but the parser matched them with case-sensitive
`string`, so any server replying with non-canonical casing (e.g. `ok`,
`* search`, `[uidvalidity ...]`, `\seen`) caused a parse error.

Adds `stringCI`/`charCI` and applies them throughout the response
parser. Along the way the tagged/fatal response handling is factored
into `pWithTaggedOrFatal`, `pDone` is split into
`pRespCode`/`pRespText`/`pStatusCode`, and string parsing is unified
(`pQuotedString`/`pLiteralString`/`pAString`/`pMailboxName`). This also:

  - surfaces an untagged `* BYE` as a fatal response instead of failing
  - accepts an empty `* SEARCH` reply (no matches)
  - accepts a `NIL` hierarchy separator in LIST/LSUB
  - stops `atomChar` from running past CR/LF

Covered by a new caseInsensitiveTest group.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11750ce623

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pWithTaggedOrFatal :: Parser RespDerivs (Either (String, Integer) b)
-> ([b] -> v)
-> Parser RespDerivs (ServerResponse, MboxUpdate, v)
pWithTaggedOrFatal lineParser build = tagged <|> fatal

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect BYE after preceding untagged lines

When a response buffer contains any untagged response before * BYE (for example, * 1 EXISTS followed by * BYE), the tagged branch consumes both lines through the generic pOtherLine fallback and then fails in pDone; the top-level fatal alternative restarts at the first line, where pFatalLine also fails. Consequently eval raises a parse error rather than returning the intended fatal BAD response. BYE detection needs to occur while processing each untagged line, not only as an alternative at the initial cursor.

Useful? React with 👍 / 👎.

char ')'
return attrs
parseSep = space >> char '"' >> anyChar `manyTill` char '"'
parseSep = space >> ((string "NIL" >> return "") <|> pQuotedString)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match NIL hierarchy separators case-insensitively

For a legal lowercase response such as * LIST () nil INBOX, this case-sensitive match makes pListLine fail; pOtherLine then consumes the response, so list silently omits the mailbox. Since the change promises case-insensitive IMAP protocol keywords, the NIL alternative should use stringCI as the surrounding LIST tokens do.

Useful? React with 👍 / 👎.

sequence $ replicate num anyChar

pAString :: Parser RespDerivs String
pAString = pQuotedString <|> pLiteralString <|> many1 atomChar

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow closing brackets in unquoted astrings

When a LIST/LSUB response contains a legal unquoted mailbox such as foo]bar, the new shared astring parser rejects it because atomChar excludes ]. IMAP's astring grammar explicitly adds the ] response-special back to its permitted characters, and the previous mailbox parser accepted every non-space character, so this regression causes the LIST row to be discarded by pOtherLine. Use a dedicated astring-character parser that permits ] rather than reusing atomChar.

Useful? React with 👍 / 👎.

Comment on lines +406 to +407
pAtomValue = do v <- many1 atomChar
return $ if map toUpper v == "NIL" then "" else v

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve scalar NIL fetch values

For a FETCH item whose value is scalar NIL, such as BODY[] NIL, the exported pFetch parser now returns an empty string instead of the previous literal "NIL". This loses the distinction between an absent value and a zero-length literal and is inconsistent with the streaming FETCH parser used by fetchByString, which preserves NIL. Keep the parsed atom unchanged and let higher-level callers interpret NIL when appropriate.

Useful? React with 👍 / 👎.

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.

1 participant