Skip to content

IMAP.status doesn't work when mailbox name contains a space#101

Merged
qnikst merged 1 commit into
qnikst:masterfrom
mpscholten:fix-status-parser
Jun 5, 2026
Merged

IMAP.status doesn't work when mailbox name contains a space#101
qnikst merged 1 commit into
qnikst:masterfrom
mpscholten:fix-status-parser

Conversation

@mpscholten

@mpscholten mpscholten commented Nov 21, 2024

Copy link
Copy Markdown
Contributor

Fixes pStatusLine so it consumes the mailbox name as either an unquoted atom or a quoted string with escaped characters before parsing the status list.

This addresses qnikst's review feedback: the previous patch handled spaces but still stopped too early when a quoted mailbox contained an escaped double quote.

Also enables the existing imap-parsers HUnit test suite in Cabal and updates the repository URL from git:// to https:// so current cabal check passes.

Regression coverage:

  • STATUS "[Gmail]/Alle Nachrichten" ...
  • STATUS "foo\" bar" ...
  • existing unquoted mailbox status response

Validation:

  • Before the parser fix, cabal test imap-parsers --test-show-details=direct failed on the quoted mailbox cases.
  • nix shell nixpkgs#cabal-install nixpkgs#ghc --command cabal test imap-parsers --test-show-details=direct
  • nix shell nixpkgs#cabal-install nixpkgs#ghc --command cabal build all
  • nix shell nixpkgs#cabal-install nixpkgs#ghc --command cabal check (only the existing CHANGELOG doc-place warning remains)
  • git diff --check

@mpscholten

Copy link
Copy Markdown
Contributor Author

@qnikst could you take a look at this? (and if you're on this maybe also the other open PR) :)

@qnikst

qnikst commented Jun 8, 2025

Copy link
Copy Markdown
Owner

Hello! Thanks for the patch, but I expect that it would need some more love, but correct me if I'm wrong.

I was not able to get through all the RFC, but I expect that in case if mailbox has special characters like spaces it MUST be quoted, and if there is already a double quote in the protocol it has to be escaped by the \ symbol. If this is correct then the current approach is only a partial solution: it does indeed fix the case of spaces, but does not fix it for the case if there are double quotes in the mailbox.

If you already have a good system to test and check the fix can you please try that case?

@qnikst

qnikst commented Jun 8, 2025

Copy link
Copy Markdown
Owner

I'd suggest using something like:

unquote `manyTill` char '"' ... where
  unquote = asum [ char '\' >> anyChar, anyChar]

This way we will be able to consume a quoted double quote and return it from the parser and will not stop before we needed.

@qnikst

qnikst commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@mpscholten looks great thanks! I'm happy to merge when undrafted :)

mpscholten commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! It is ready for review now :)

@qnikst qnikst merged commit ee75eb5 into qnikst:master Jun 5, 2026
21 checks passed
@qnikst

qnikst commented Jun 5, 2026

Copy link
Copy Markdown
Owner

@mpscholten thanks for the work and patience.

I'll try to make a release (but possibly will be able to only after/or during Zurihac)

@mpscholten

Copy link
Copy Markdown
Contributor Author

thanks 👍 I have some more patches coming

Will be at Zurihac tomorrow as well (already at HIW right now)

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.

2 participants