Verificaition emails "%" sign escape problem

[saroyanm]

Background

After Microsoft Edge subscription page creation (#2843), we have noticed that verification link is broken after redirection from some email management software (ex.: Outlook).
The problem is because of "%" sign being escaped to "%25", with #3180 we made a quick fix, which needs to be reverted after fixing current ticket.

What to change

Proceed values of email parameter like example%2540example.com in /verifyEmail service, or send verification email with two parameters user and domain (ex.: /verifyEmail?user=exampl&domain=example.com&signature=...&product=edge&lang=en)

[saroyanm]

@Dave @Sebastian probably description needs to be updated, but I hope I could describe the problem.

[sebastian]

Before taking any steps, like suggested in this issue, we should further investigate that issue. Is the @-sign the only effected character? Does it also happen when using a different transfer encoding (e.g. base64) or when using HTML? As this seems to be a bug in Outlook, is it known to Microsoft, and documented somewhere? Or does it effect other mail clients as well? And either way, what is causing it?

[saroyanm]

Replying to sebastian:

Before taking any steps, like suggested in this issue, we should further investigate that issue. Is the @-sign the only effected character?

I guess this affects all "%" Characters, because of double URI encoding, see the modified URL by Outlook below:
https://adblockplus.org/verifyEmail?email=oleksandr%2540adblockplus.org&signature=9ba8f03cd89fa08168c723c900a7c06766e5f981&product=edge&lang=en

As this seems to be a bug in Outlook, is it known to Microsoft, and documented somewhere?

Couldn't find any bug filed on Microsoft side.

[matze]

It does affect all encoded characters; Ollie tested and confirmed that before.

[sebastian]

Hmm, I couldn't find any related known bug either. We should file one.

For the time being, I suggest following workaround:

  email = params.get('email', '')
  signature = params.get('signature', '')
  if sign(config, email) != signature: 
    # HACK: MS Outlook double-encodes URLs
    email = urllib.unquote(email)

    if sign(config, email) != signature:          
      return send_simple_response(                
        start_response, 403,
        'Invalid signature in verification request.'
      )

[saroyanm]

Replying to sebastian:

Hmm, I couldn't find any related known bug either. We should file one.

hmm, they even do not have proper place to file a bug, the only thing I found is that you can ask question to community. @Ollie, do you know is it the way of filing a bug - asking community ?

For the time being, I suggest following workaround:

  email = params.get('email', '')
  signature = params.get('signature', '')
  if sign(config, email) != signature: 
    # HACK: MS Outlook double-encodes URLs
    email = urllib.unquote(email)

    if sign(config, email) != signature:          
      return send_simple_response(                
        start_response, 403,
        'Invalid signature in verification request.'
      )

Looks good.

[oleksandr]

1. The bug is in the built in Mail app of Windows 10 (it isn't called Outlook)
2. Issues are reported there using the Windows 10 built in Feedback tool.
3. The issue was already reported by someone 2 months ago (see attachment). It causes problems with all sorts of links (for example https://issues.adblockplus.org/ticket/3189#comment:6 fails as well, translating to https://issues.adblockplus.org/ticket/3189%23comment:6). So I would assume it will get fixed soon. I'll test the latest Fast Ring version.

[sebastian]

Good to know. Assuming this bug is matter to be fixed on their end, I'd rather not going to address it in our WSGI handler, also given that we already have a workaround in the server configuration in place. However, should this issue persist, I am in the favor of replacing the current work around with the one suggested above.

[saroyanm]

Replying to sebastian:

However, should this issue persist, I am in the favor of replacing the current work around with the one suggested above.

Agree, I can prepare a patch after this issue is ready.

[sebastian]

Any update on the progress on Microsoft's end to fix that bug?

[kvas]

Dear stakeholders of this ticket,

I'm cleaning up Sitescripts tickets in Trac due to its phase out. This is one of the tickets that I could not myself close or move, so I need your input on it.

Please let me know if this ticket is still relevant for you and we can then discuss where it should be moved. If you think that this ticket is no longer relevant, you can write a comment explaining this or just ignore this message.

If I see now comments in the ticket on October 14, I will close it as rejected.

Best regards,
Vasily