invalid flag-list #228

Eguthrie3214 · 2023-11-13T23:12:14Z

After updating from 0.4.1 to 0.4.3 we noticed errors when running a test that selects an inbox

rails aborted!
Net::IMAP::ResponseParseError: invalid flag-list
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser/parser_utils.rb:234:in parse_error' /Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser/parser_utils.rb:205:in match_re'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:1713:in flag_list' /Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:1155:in mailbox_data__flags'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:571:in `response_data'

I was able to track this down to #212 and the change to flag parsing.

Given the same input of

for both versions, 0.4.3 fails

If it helps the combo of services im using looks like this

version: '3'

services:

  db:
    image: postgres
    ports:
      - "6099:5432"
    environment:
      - POSTGRES_USER=workflows
      - POSTGRES_PASSWORD=password
    volumes:
      - postgres-data:/var/lib/postgresql/data

  redis:
    image: redis
    ports:
      - "7099:6379"

  mail:
    image: greenmail/standalone
    ports:
      - "3465:3465" #SMTPS
      - "3993:3993" #IMAPS
      - "3025:3025" #SMTP
      - "3143:3143" #IMAP
  roundcube:
    image: roundcube/roundcubemail:latest
    depends_on:
      - mail
    ports:
      - "8000:80"
    environment:
      - ROUNDCUBEMAIL_DEFAULT_HOST=mail  # IMAP server - tls:// prefix for STARTTLS, ssl:// for SSL/TLS
      - ROUNDCUBEMAIL_DEFAULT_PORT=3143               # IMAP port
      - ROUNDCUBEMAIL_SMTP_SERVER=mail  # SMTP server - tls:// prefix for STARTTLS, ssl:// for SSL/TLS
      - ROUNDCUBEMAIL_SMTP_PORT=3025
  dynamodb:
    image: amazon/dynamodb-local
    ports:
      - "8001:8000"
    volumes:
      - dynamodb-data:/home/dynamodblocal/data
    command: "-jar DynamoDBLocal.jar -sharedDb -dbPath /home/dynamodblocal/data/"

volumes:
  postgres-data:
  dynamodb-data:

and for an example of what im running to trigger this

    puts imap.list("", "*")
    imap.select("INBOX")

imap is an instance of the imap client
The list command works, but it breaks during the select.

Thank you for the time and I would be happy to provide more information!

The text was updated successfully, but these errors were encountered:

nevans · 2023-11-15T08:57:51Z

Sorry about this! I do have a few questions:

Is there a missing "\r" in the example screenshot? Because "* LIST () \".\" \"INBOX\"\r\n" (with the \r) parses just fine for me, but "* LIST () \".\" \"INBOX\"\n" (without it) is expected to fail. But also, the @str in that screenshot doesn't seem to match the stacktrace. That stacktrace seems to match a FLAGS response of some sort.

If you use Net::IMAP.debug = true, it should print out more useful information whenever a parse error is encountered. Could you copy/paste that debug result here, please?

Eguthrie3214 · 2023-11-15T22:50:38Z

Hello, sorry about any confusion, I took screen shots of both the input when it worked and when it didnt so I may have gotten them mixed up. Here is a trace with the debug option enabled.


Net::IMAP::ResponseParser parse_error: invalid flag-list
  tokenized : "* FLAGS "
  remaining : "(\\Answered \\Deleted \\Draft \\Flagged \\Seen \\*)\r\n"
  @lex_state: EXPR_BEG
  @pos      : 8
  @token    : nil
  caller[ 0]: match_re                       (parser_utils:205)
  caller[ 1]: flag_list                      (response_parser:1713)
  caller[ 2]: mailbox_data__flags            (response_parser:1155)
  caller[ 3]: response_data                  (response_parser:571)
  caller[ 4]: response                       (response_parser:509)
  caller[ 5]: parse                          (response_parser:33)
  caller[ 6]: get_response                   (imap:2544)
  caller[ 7]: receive_responses              (imap:2439)
  caller[ 8]: start_receiver_thread          (imap:2416)
rails aborted!
Net::IMAP::ResponseParseError: invalid flag-list
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser/parser_utils.rb:234:in `parse_error'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser/parser_utils.rb:205:in `match_re'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:1713:in `flag_list'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:1155:in `mailbox_data__flags'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:571:in `response_data'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:509:in `response'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap/response_parser.rb:33:in `parse'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap.rb:2544:in `get_response'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap.rb:2439:in `receive_responses'
/Users/evanguthrie/.rvm/gems/ruby-3.1.0@v3-workflows/gems/net-imap-0.4.3/lib/net/imap.rb:2416:in `block in start_receiver_thread'

If I break on line 201 of parser_utils.rb I get

Let me know if you want me to do anything else and I would be happy to do it :) This isn't currently an issue as we've locked our gem to 0.4.1, but it would be nice to able to upgrade.

nevans · 2023-11-16T09:01:25Z

Thanks.

So this looks like a bug in the server: while \* is valid for PERMANENTFLAGS, it is not for FLAGS. As part of updating the parser code for rev2 and various LIST extensions, I had changed it to be more strict about this.

While I'd like to parse strictly when we can get away with it, we do include many workarounds for "quirky" servers. I'll probably want to add configuration params in the future, but for now, what do you think about printing a warning message for this sort of invalid response?

Eguthrie3214 · 2023-11-16T16:17:12Z

I think that would be great. I work with email quite a bit (as do you it seems) and its almost always problems of like "x client or server does this, and thats incompatible with y" so I feel like being somewhat permissive, or giving an option to configure if needed would be cool.

Thanks so much for the help on this!

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans · 2023-11-17T16:39:48Z

@Eguthrie3214 I pushed a branch with a fix, and I made an issue with greenmail. That project seems to be active and responsive, so I want to hear their response before I turn the branch into a PR (and merge) But, if it's useful, you can test or use the branch immediately (e.g. using bundler: gem "net-imap", github:, branch:).

My workaround (the branch contains some tangentially related changes... it will probably be rebased and updated): https://github.com/ruby/net-imap/compare/workaround-invalid-flag
greenmail issue: \* is not a valid IMAP flag greenmail-mail-test/greenmail#633

Eguthrie3214 · 2023-11-17T17:57:59Z

Thank you so much! Ive tested the branch and it works. Ill keep track of your greenmail issue as well because the best solution is probably them just making the mail client adhere to the rfc.

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans · 2023-11-29T23:54:31Z

@Eguthrie3214 FYI: I'm not really keeping the workaround branch up-to-date. But I won't delete it either, at least not any time soon. The greenmail issue was closed and marked to be backported, but I don't know how often they make new releases for backports.

I'm going to close the ticket. Please reopen it if you think it should be kept open or addressed. I'll reopen if I see any evidence of servers in the wild sending a response like that.

nevans · 2023-12-06T19:38:03Z

Aaaaaand Gmail has a bug similar to this one! 😆 It's slightly different (worse actually), so I'm opening another issue for it: #241.

The workaround for #241 (PR #246) also applies to #228. I haven't seen any evidence of any "real" servers with this exact error yet. And the upstream issue (greenmail-mail-test/greenmail#633) was fixed promptly (thanks!). So I don't feel that it's critical to be compatible with it... But we _do_ need this workaround for #241. So it makes sense to at least document this issue in our test fixtures, for posterity.

The workaround for #241 (PR #246) also applies to #228. The upstream issue (greenmail-mail-test/greenmail#633) was fixed promptly (thanks!). Also, greenmail is a testing fake server and I haven't seen any evidence of any "real" servers with this exact error yet. So I don't feel that it's critical to be compatible with it... But we _do_ need this workaround for #241. So it makes sense to at least document this issue in our test fixtures, for posterity.

nevans · 2023-12-12T14:25:59Z

@Eguthrie3214 For what it's worth, PR #246 to fix #241 was released with v0.4.8, and it does work around this issue too.

Even though greenmail fixed their bug and I haven't seen it anywhere else, I figured that it was at least worth capturing this scenario in our test suite. So I just merged #249 to do that. 🙂

nevans added a commit that referenced this issue Nov 17, 2023

🩹 Add workaround for invalid \* flag

a078cc4

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans added a commit that referenced this issue Nov 20, 2023

🩹 Add workaround for invalid \* flag

bb6ef3b

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans added a commit that referenced this issue Nov 20, 2023

🩹 Add workaround for invalid \* flag

ccd6d31

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans added a commit that referenced this issue Nov 20, 2023

🩹 Add workaround for invalid \* flag

3bdef18

* Motivated by greenmail-mail-test/greenmail#633. * Fixes #228.

nevans closed this as completed Nov 29, 2023

nevans mentioned this issue Dec 12, 2023

✅ Document and test workaround for invalid "\*" in FLAGS #249

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

invalid flag-list #228

invalid flag-list #228

Eguthrie3214 commented Nov 13, 2023

nevans commented Nov 15, 2023 •

edited

Loading

Eguthrie3214 commented Nov 15, 2023

nevans commented Nov 16, 2023 •

edited

Loading

Eguthrie3214 commented Nov 16, 2023

nevans commented Nov 17, 2023 •

edited

Loading

Eguthrie3214 commented Nov 17, 2023

nevans commented Nov 29, 2023 •

edited

Loading

nevans commented Dec 6, 2023 •

edited

Loading

nevans commented Dec 12, 2023 •

edited

Loading

invalid flag-list #228

invalid flag-list #228

Comments

Eguthrie3214 commented Nov 13, 2023

nevans commented Nov 15, 2023 • edited Loading

Eguthrie3214 commented Nov 15, 2023

nevans commented Nov 16, 2023 • edited Loading

Eguthrie3214 commented Nov 16, 2023

nevans commented Nov 17, 2023 • edited Loading

Eguthrie3214 commented Nov 17, 2023

nevans commented Nov 29, 2023 • edited Loading

nevans commented Dec 6, 2023 • edited Loading

nevans commented Dec 12, 2023 • edited Loading

nevans commented Nov 15, 2023 •

edited

Loading

nevans commented Nov 16, 2023 •

edited

Loading

nevans commented Nov 17, 2023 •

edited

Loading

nevans commented Nov 29, 2023 •

edited

Loading

nevans commented Dec 6, 2023 •

edited

Loading

nevans commented Dec 12, 2023 •

edited

Loading