Skip to content

Fix address with Q-encoded display name containing double quotes #841

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rafbm
Copy link
Contributor

@rafbm rafbm commented Dec 4, 2014

I added a spec for parsing the following address:

=?ISO-8859-1?Q?"Jan_Kr=FCtisch"?= <[email protected]>

Before the fix, display name was parsed as Jan_Kr=FCtisch because when a quoted string was found (ie. "Jan_Kr=FCtisch"), it was used as the display name and the surrounding characters were ignored.

After the fix, display name is properly encoded as Jan Krütisch. Code-wise, the qstr variable is now used as the display name instead of phrase only if qstr is equal to phrase minus the quotes.

I would have liked to avoid creating the %("#{qstr}") string, but I did not see another way. Other than that performance/memory should not be affected.

@bf4
Copy link
Contributor

bf4 commented Dec 14, 2014

@grosser any concerns?

@grosser
Copy link
Contributor

grosser commented Dec 14, 2014

hmm old logic sounds broken, more logic does not feel very clean either ... but at least we have a test ... so choosing between the two evils I'd pick the new code ... 👍

@rafbm
Copy link
Contributor Author

rafbm commented Dec 14, 2014

@grosser You are totally right that this is likely not the most appropriate fix. I just did a quick test not using the qstr variable at all, just using the phrase variable, stripping leading and trailing ", all tests still pass:

        when :angle_addr_s
          if phrase_e
            phrase = s[phrase_s..phrase_e].strip
            if phrase[0] == '"' && phrase[-1] == '"'
              phrase = phrase[1..-2]
            end
            address.display_name = phrase
            phrase_e = phrase_s = nil
          end

This questions the relevance of parsing quoted string as a token.

@grosser
Copy link
Contributor

grosser commented Dec 14, 2014

I like that version more ... are there enough tests that you are comfortable with that change ?

@rafbm
Copy link
Contributor Author

rafbm commented Feb 5, 2015

Sorry for the delay!

Yes I am confident enough with the specs I just added for comments inside Q-encoded display names.

@rafbm
Copy link
Contributor Author

rafbm commented Feb 5, 2015

Mmh not sure why specs fail on Travis. All green on my machine. Can someone have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants