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

🩹 Workaround invalid Gmail FLAGS response #246

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented 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 #212, the ResponseParser simply grabbed everything inside the parentheses and then scanned through with a very liberal "flag" regexp. This PR attempts to parse it strictly at first and if that fails falls back to the "quirks mode" parser, similar to the original.

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 nevans force-pushed the workaround/gmail-invalid-flags branch from 8506491 to 3f88747 Compare December 12, 2023 03:17
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 nevans force-pushed the workaround/gmail-invalid-flags branch from 3f88747 to 20b0031 Compare December 12, 2023 04:00
@nevans nevans merged commit cc19514 into ruby:master Dec 12, 2023
11 checks passed
@nevans nevans deleted the workaround/gmail-invalid-flags branch December 12, 2023 04:03
nevans added a commit that referenced this pull request 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 pull request 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 pull request 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 pull request 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 nevans mentioned this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

GMail allows flags with invalid atom-specials characters to be created
1 participant