Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GMail allows flags with invalid atom-specials characters to be created #241

Closed
nevans opened this issue Dec 6, 2023 · 0 comments · Fixed by #246
Closed

GMail allows flags with invalid atom-specials characters to be created #241

nevans opened this issue Dec 6, 2023 · 0 comments · Fixed by #246

Comments

@nevans
Copy link
Collaborator

nevans commented Dec 6, 2023

Not only does GMail allow flags with invalid atom-specials characters to be created, it will then it will print those flags in the FLAGS response and PERMANENT FLAGS response data. This causes Net::IMAP::ResponseParser to crash upon selecting the offending mailbox.

Our earlier parser was a bit more forgiving, but #212 gave a small performance boost (in addition to parsing more strictly). So this has only been a problem since

The bug has been there for a decade or more, so I doubt they'll fix it any time soon:

However, it looks to me like no one ever filed a public bugreport in the Gmail bugtracker, so... I filed a bug-report with more details here: https://issuetracker.google.com/issues/315160951. Maybe now they'll make some changes to fix it?

We can work around the invalid "]" with a hack: parse up to the first that is followed by SP or CRLF and then simply split the contents into an array. But I don't think there's any way to unambiguously parse flags containing an invalid SP! Alternatively, when we witness an error like this, we could give up on correctly parsing the flags and simply provide the user with some UnparsedData.

In order to preserve the performance boost from #212 for well-behaved servers, the workaround could be implemented after first attempting to parse it the correct way... possible implementing the workarounds in a rescue clause.

nevans added a commit to nevans/net-imap that referenced this issue Dec 11, 2023
Gmail allows (or allowed) users to create flags containing some invalid
`atom-specials` chars, such as `"]"` or `SP`.  Ultimately, this isn't
unambiguously parsable.  But we can workaround it.

Prior to ruby#212, the ResponseParser simply grabbed everything inside the
parentheses and then scanned through with a very liberal "flag" regexp.
This commit attempts to parse it strictly at first and then falls back
to the "quirks mode" parser if that fails.

Fixes ruby#241.

This also uses `delete_prefix!` to avoid another string allocation when
converting flag strings into symbols.

-----
Benchmark results:
```
Calculating -------------------------------------
                                                 v0.4.7-5-gbb6ced5-dirty      0.4.2      0.4.7
                         fetch_msg_att_flags_and_uid             52.640k   33.016k   48.638k i/s -  156.968k times in 2.981890s 4.754293s 3.227301s
                       flags_with_various_flag_types             87.683k   66.967k   87.213k i/s -  285.657k times in 3.257832s 4.265654s 3.275385s
imap.gmail.com allows invalid atom-specials in flags             30.170k   38.268k     ERROR i/s -   98.149k times in 3.253235s 2.564783s 0.000000s
              list_with_various_flag_capitalizations             38.377k   31.769k   37.976k i/s -  122.317k times in 3.187235s 3.850240s 3.220918s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example             55.032k   46.788k   55.223k i/s -  175.994k times in 3.198034s 3.761512s 3.186992s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example             61.554k   55.361k   62.390k i/s -  190.839k times in 3.100343s 3.447147s 3.058791s
    resp_text_PERMANENTFLAGS_with_various_flag_types             53.750k   44.196k   54.118k i/s -  172.657k times in 3.212227s 3.906645s 3.190398s
                rfc3501_7.2.6_FLAGS_response_example             79.240k   61.943k   78.187k i/s -  256.311k times in 3.234620s 4.137845s 3.278185s

Comparison:
                         fetch_msg_att_flags_and_uid
                v0.4.7-5-gbb6ced5-dirty:     52640.4 i/s
                                  0.4.7:     48637.5 i/s - 1.08x  slower
                                  0.4.2:     33016.1 i/s - 1.59x  slower

                       flags_with_various_flag_types
                v0.4.7-5-gbb6ced5-dirty:     87683.2 i/s
                                  0.4.7:     87213.3 i/s - 1.01x  slower
                                  0.4.2:     66966.8 i/s - 1.31x  slower

imap.gmail.com allows invalid atom-specials in flags
                                  0.4.2:     38268.0 i/s
                v0.4.7-5-gbb6ced5-dirty:     30169.7 i/s - 1.27x  slower
                                  0.4.7:         0.0 i/s

              list_with_various_flag_capitalizations
                v0.4.7-5-gbb6ced5-dirty:     38377.2 i/s
                                  0.4.7:     37975.8 i/s - 1.01x  slower
                                  0.4.2:     31768.7 i/s - 1.21x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example
                                  0.4.7:     55222.6 i/s
                v0.4.7-5-gbb6ced5-dirty:     55031.9 i/s - 1.00x  slower
                                  0.4.2:     46788.1 i/s - 1.18x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example
                                  0.4.7:     62390.3 i/s
                v0.4.7-5-gbb6ced5-dirty:     61554.2 i/s - 1.01x  slower
                                  0.4.2:     55361.4 i/s - 1.13x  slower

    resp_text_PERMANENTFLAGS_with_various_flag_types
                                  0.4.7:     54117.7 i/s
                v0.4.7-5-gbb6ced5-dirty:     53749.9 i/s - 1.01x  slower
                                  0.4.2:     44195.7 i/s - 1.22x  slower

                rfc3501_7.2.6_FLAGS_response_example
                v0.4.7-5-gbb6ced5-dirty:     79239.9 i/s
                                  0.4.7:     78186.9 i/s - 1.01x  slower
                                  0.4.2:     61943.1 i/s - 1.28x  slower
```
nevans added a commit to nevans/net-imap that referenced this issue Dec 12, 2023
Gmail allows (or allowed) users to create flags containing some invalid
`atom-specials` chars, such as `"]"` or `SP`.  Ultimately, this isn't
unambiguously parsable.  But we can workaround it.

Prior to ruby#212, the ResponseParser simply grabbed everything inside the
parentheses and then scanned through with a very liberal "flag" regexp.
This commit attempts to parse it strictly at first and then falls back
to the "quirks mode" parser if that fails.

Fixes ruby#241.

This also uses `delete_prefix!` to avoid another string allocation when
converting flag strings into symbols.

-----
Benchmark results:
```
Calculating -------------------------------------
                                                 v0.4.7-5-gbb6ced5-dirty      0.4.2      0.4.7
                         fetch_msg_att_flags_and_uid             52.640k   33.016k   48.638k i/s -  156.968k times in 2.981890s 4.754293s 3.227301s
                       flags_with_various_flag_types             87.683k   66.967k   87.213k i/s -  285.657k times in 3.257832s 4.265654s 3.275385s
imap.gmail.com allows invalid atom-specials in flags             30.170k   38.268k     ERROR i/s -   98.149k times in 3.253235s 2.564783s 0.000000s
              list_with_various_flag_capitalizations             38.377k   31.769k   37.976k i/s -  122.317k times in 3.187235s 3.850240s 3.220918s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example             55.032k   46.788k   55.223k i/s -  175.994k times in 3.198034s 3.761512s 3.186992s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example             61.554k   55.361k   62.390k i/s -  190.839k times in 3.100343s 3.447147s 3.058791s
    resp_text_PERMANENTFLAGS_with_various_flag_types             53.750k   44.196k   54.118k i/s -  172.657k times in 3.212227s 3.906645s 3.190398s
                rfc3501_7.2.6_FLAGS_response_example             79.240k   61.943k   78.187k i/s -  256.311k times in 3.234620s 4.137845s 3.278185s

Comparison:
                         fetch_msg_att_flags_and_uid
                v0.4.7-5-gbb6ced5-dirty:     52640.4 i/s
                                  0.4.7:     48637.5 i/s - 1.08x  slower
                                  0.4.2:     33016.1 i/s - 1.59x  slower

                       flags_with_various_flag_types
                v0.4.7-5-gbb6ced5-dirty:     87683.2 i/s
                                  0.4.7:     87213.3 i/s - 1.01x  slower
                                  0.4.2:     66966.8 i/s - 1.31x  slower

imap.gmail.com allows invalid atom-specials in flags
                                  0.4.2:     38268.0 i/s
                v0.4.7-5-gbb6ced5-dirty:     30169.7 i/s - 1.27x  slower
                                  0.4.7:         0.0 i/s

              list_with_various_flag_capitalizations
                v0.4.7-5-gbb6ced5-dirty:     38377.2 i/s
                                  0.4.7:     37975.8 i/s - 1.01x  slower
                                  0.4.2:     31768.7 i/s - 1.21x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example
                                  0.4.7:     55222.6 i/s
                v0.4.7-5-gbb6ced5-dirty:     55031.9 i/s - 1.00x  slower
                                  0.4.2:     46788.1 i/s - 1.18x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example
                                  0.4.7:     62390.3 i/s
                v0.4.7-5-gbb6ced5-dirty:     61554.2 i/s - 1.01x  slower
                                  0.4.2:     55361.4 i/s - 1.13x  slower

    resp_text_PERMANENTFLAGS_with_various_flag_types
                                  0.4.7:     54117.7 i/s
                v0.4.7-5-gbb6ced5-dirty:     53749.9 i/s - 1.01x  slower
                                  0.4.2:     44195.7 i/s - 1.22x  slower

                rfc3501_7.2.6_FLAGS_response_example
                v0.4.7-5-gbb6ced5-dirty:     79239.9 i/s
                                  0.4.7:     78186.9 i/s - 1.01x  slower
                                  0.4.2:     61943.1 i/s - 1.28x  slower
```
nevans added a commit to nevans/net-imap that referenced this issue Dec 12, 2023
Gmail allows (or allowed) users to create flags containing some invalid
`atom-specials` chars, such as `"]"` or `SP`.  Ultimately, this isn't
unambiguously parsable.  But we can workaround it.

Prior to ruby#212, the ResponseParser simply grabbed everything inside the
parentheses and then scanned through with a very liberal "flag" regexp.
This commit attempts to parse it strictly at first and then falls back
to the "quirks mode" parser if that fails.

Fixes ruby#241.

This also uses `delete_prefix!` to avoid another string allocation when
converting flag strings into symbols.

-----
Benchmark results:
```
Calculating -------------------------------------
                                                     v0.4.7-6-g3f88747    0.2.3    0.3.7    0.4.2    0.4.7
                         fetch_msg_att_flags_and_uid           53.343k  33.978k  33.562k  34.063k  52.615k i/s - 162.079k times in 3.038415s 4.770048s 4.829303s 4.758267s 3.080481s
                       flags_with_various_flag_types           95.497k  73.549k  73.802k  73.777k  93.892k i/s - 289.950k times in 3.036226s 3.942294s 3.928770s 3.930078s 3.088128s
imap.gmail.com allows invalid atom-specials in flags           32.155k  41.518k  38.704k  37.842k    ERROR i/s - 100.595k times in 3.128400s 2.422950s 2.599085s 2.658261s 0.000000s
              list_with_various_flag_capitalizations           42.105k  48.496k  45.108k  31.679k  39.903k i/s - 118.998k times in 2.826218s 2.453753s 2.638075s 3.756420s 2.982145s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example           56.608k  44.082k  44.029k  46.168k  56.743k i/s - 184.142k times in 3.252913s 4.177284s 4.182247s 3.988496s 3.245177s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example           62.031k  47.918k  49.941k  56.477k  63.410k i/s - 197.518k times in 3.184188s 4.122017s 3.955044s 3.497315s 3.114928s
    resp_text_PERMANENTFLAGS_with_various_flag_types           55.224k  41.582k  41.547k  46.085k  54.984k i/s - 178.322k times in 3.229081s 4.288422s 4.292034s 3.869394s 3.243159s
                rfc3501_7.2.6_FLAGS_response_example           79.586k  61.576k  62.134k  62.640k  79.194k i/s - 268.613k times in 3.375129s 4.362270s 4.323131s 4.288183s 3.391847s

Comparison:
                         fetch_msg_att_flags_and_uid
                      v0.4.7-6-g3f88747:     53343.3 i/s
                                  0.4.7:     52614.8 i/s - 1.01x  slower
                                  0.4.2:     34062.6 i/s - 1.57x  slower
                                  0.2.3:     33978.5 i/s - 1.57x  slower
                                  0.3.7:     33561.6 i/s - 1.59x  slower

                       flags_with_various_flag_types
                      v0.4.7-6-g3f88747:     95496.8 i/s
                                  0.4.7:     93891.8 i/s - 1.02x  slower
                                  0.3.7:     73801.7 i/s - 1.29x  slower
                                  0.4.2:     73777.2 i/s - 1.29x  slower
                                  0.2.3:     73548.5 i/s - 1.30x  slower

imap.gmail.com allows invalid atom-specials in flags
                                  0.2.3:     41517.6 i/s
                                  0.3.7:     38704.0 i/s - 1.07x  slower
                                  0.4.2:     37842.4 i/s - 1.10x  slower
                      v0.4.7-6-g3f88747:     32155.4 i/s - 1.29x  slower
                                  0.4.7:         0.0 i/s

              list_with_various_flag_capitalizations
                                  0.2.3:     48496.3 i/s
                                  0.3.7:     45107.9 i/s - 1.08x  slower
                v0.4.7-6-gd7bf81d-dirty:     42105.0 i/s - 1.15x  slower
                                  0.4.7:     39903.5 i/s - 1.22x  slower
                                  0.4.2:     31678.6 i/s - 1.53x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example
                                  0.4.7:     56743.3 i/s
                      v0.4.7-6-g3f88747:     56608.3 i/s - 1.00x  slower
                                  0.4.2:     46168.3 i/s - 1.23x  slower
                                  0.2.3:     44081.8 i/s - 1.29x  slower
                                  0.3.7:     44029.4 i/s - 1.29x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example
                                  0.4.7:     63410.1 i/s
                      v0.4.7-6-g3f88747:     62030.9 i/s - 1.02x  slower
                                  0.4.2:     56477.0 i/s - 1.12x  slower
                                  0.3.7:     49940.8 i/s - 1.27x  slower
                                  0.2.3:     47917.8 i/s - 1.32x  slower

    resp_text_PERMANENTFLAGS_with_various_flag_types
                      v0.4.7-6-g3f88747:     55223.8 i/s
                                  0.4.7:     54984.0 i/s - 1.00x  slower
                                  0.4.2:     46085.3 i/s - 1.20x  slower
                                  0.2.3:     41582.2 i/s - 1.33x  slower
                                  0.3.7:     41547.2 i/s - 1.33x  slower

                rfc3501_7.2.6_FLAGS_response_example
                      v0.4.7-6-g3f88747:     79586.0 i/s
                                  0.4.7:     79193.7 i/s - 1.00x  slower
                                  0.4.2:     62640.3 i/s - 1.27x  slower
                                  0.3.7:     62133.9 i/s - 1.28x  slower
                                  0.2.3:     61576.4 i/s - 1.29x  slower
```
nevans added a commit that referenced this issue Dec 12, 2023
Gmail allows (or allowed) users to create flags containing some invalid
`atom-specials` chars, such as `"]"` or `SP`.  Ultimately, this isn't
unambiguously parsable.  But we can workaround it.

Prior to #212, the ResponseParser simply grabbed everything inside the
parentheses and then scanned through with a very liberal "flag" regexp.
This commit attempts to parse it strictly at first and then falls back
to the "quirks mode" parser if that fails.

Fixes #241.

This also uses `delete_prefix!` to avoid another string allocation when
converting flag strings into symbols.

-----
Benchmark results:
```
Calculating -------------------------------------
                                                     v0.4.7-6-g3f88747    0.2.3    0.3.7    0.4.2    0.4.7
                         fetch_msg_att_flags_and_uid           53.343k  33.978k  33.562k  34.063k  52.615k i/s - 162.079k times in 3.038415s 4.770048s 4.829303s 4.758267s 3.080481s
                       flags_with_various_flag_types           95.497k  73.549k  73.802k  73.777k  93.892k i/s - 289.950k times in 3.036226s 3.942294s 3.928770s 3.930078s 3.088128s
imap.gmail.com allows invalid atom-specials in flags           32.155k  41.518k  38.704k  37.842k    ERROR i/s - 100.595k times in 3.128400s 2.422950s 2.599085s 2.658261s 0.000000s
              list_with_various_flag_capitalizations           42.105k  48.496k  45.108k  31.679k  39.903k i/s - 118.998k times in 2.826218s 2.453753s 2.638075s 3.756420s 2.982145s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example           56.608k  44.082k  44.029k  46.168k  56.743k i/s - 184.142k times in 3.252913s 4.177284s 4.182247s 3.988496s 3.245177s
      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example           62.031k  47.918k  49.941k  56.477k  63.410k i/s - 197.518k times in 3.184188s 4.122017s 3.955044s 3.497315s 3.114928s
    resp_text_PERMANENTFLAGS_with_various_flag_types           55.224k  41.582k  41.547k  46.085k  54.984k i/s - 178.322k times in 3.229081s 4.288422s 4.292034s 3.869394s 3.243159s
                rfc3501_7.2.6_FLAGS_response_example           79.586k  61.576k  62.134k  62.640k  79.194k i/s - 268.613k times in 3.375129s 4.362270s 4.323131s 4.288183s 3.391847s

Comparison:
                         fetch_msg_att_flags_and_uid
                      v0.4.7-6-g3f88747:     53343.3 i/s
                                  0.4.7:     52614.8 i/s - 1.01x  slower
                                  0.4.2:     34062.6 i/s - 1.57x  slower
                                  0.2.3:     33978.5 i/s - 1.57x  slower
                                  0.3.7:     33561.6 i/s - 1.59x  slower

                       flags_with_various_flag_types
                      v0.4.7-6-g3f88747:     95496.8 i/s
                                  0.4.7:     93891.8 i/s - 1.02x  slower
                                  0.3.7:     73801.7 i/s - 1.29x  slower
                                  0.4.2:     73777.2 i/s - 1.29x  slower
                                  0.2.3:     73548.5 i/s - 1.30x  slower

imap.gmail.com allows invalid atom-specials in flags
                                  0.2.3:     41517.6 i/s
                                  0.3.7:     38704.0 i/s - 1.07x  slower
                                  0.4.2:     37842.4 i/s - 1.10x  slower
                      v0.4.7-6-g3f88747:     32155.4 i/s - 1.29x  slower
                                  0.4.7:         0.0 i/s

              list_with_various_flag_capitalizations
                                  0.2.3:     48496.3 i/s
                                  0.3.7:     45107.9 i/s - 1.08x  slower
                v0.4.7-6-gd7bf81d-dirty:     42105.0 i/s - 1.15x  slower
                                  0.4.7:     39903.5 i/s - 1.22x  slower
                                  0.4.2:     31678.6 i/s - 1.53x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.1_example
                                  0.4.7:     56743.3 i/s
                      v0.4.7-6-g3f88747:     56608.3 i/s - 1.00x  slower
                                  0.4.2:     46168.3 i/s - 1.23x  slower
                                  0.2.3:     44081.8 i/s - 1.29x  slower
                                  0.3.7:     44029.4 i/s - 1.29x  slower

      resp_code_PERMANENTFLAGS_rfc3501_6.3.2_example
                                  0.4.7:     63410.1 i/s
                      v0.4.7-6-g3f88747:     62030.9 i/s - 1.02x  slower
                                  0.4.2:     56477.0 i/s - 1.12x  slower
                                  0.3.7:     49940.8 i/s - 1.27x  slower
                                  0.2.3:     47917.8 i/s - 1.32x  slower

    resp_text_PERMANENTFLAGS_with_various_flag_types
                      v0.4.7-6-g3f88747:     55223.8 i/s
                                  0.4.7:     54984.0 i/s - 1.00x  slower
                                  0.4.2:     46085.3 i/s - 1.20x  slower
                                  0.2.3:     41582.2 i/s - 1.33x  slower
                                  0.3.7:     41547.2 i/s - 1.33x  slower

                rfc3501_7.2.6_FLAGS_response_example
                      v0.4.7-6-g3f88747:     79586.0 i/s
                                  0.4.7:     79193.7 i/s - 1.00x  slower
                                  0.4.2:     62640.3 i/s - 1.27x  slower
                                  0.3.7:     62133.9 i/s - 1.28x  slower
                                  0.2.3:     61576.4 i/s - 1.29x  slower
```
nevans added a commit that referenced this issue Dec 12, 2023
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.
nevans added a commit that referenced this issue Dec 12, 2023
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 added a commit that referenced this issue Dec 12, 2023
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 added a commit that referenced this issue Dec 12, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant