Page MenuHomeSoftware Heritage

fix mailto encoding for inbound email
ClosedPublic

Authored by bchauvet on May 19 2022, 9:22 AM.

Diff Detail

Repository
rDWAPPS Web applications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 29479
Build 46065: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 46064: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D7859 (id=28370)

Rebasing onto b548d2f261...

Current branch diff-target is up to date.
Changes applied before test
commit ffd8a035e2cbf12f109ab3af45828bc31fc575d7
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Thu May 19 09:20:36 2022 +0200

    fix mailto econding for inboud email

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1851/ for more details.

jayeshv added inline comments.
assets/src/bundles/add_forge/request-dashboard.js
60

Isn't this needed in the previous line (59) as well?

UrlEncoded all email addresses

good catch!

assets/src/bundles/add_forge/request-dashboard.js
136

IMO, URL-encoding should be here, where we actually build the URL.

text + emailTo and emailCc aren't necessarily meant only to be used for building URLs

*thumbs up*

Beware your commit (and as a result your diff description) though.

It should be:
Fix mailto e*n*co**ding for inbo*u*nd email

And please add the Related to T4260 in the diff description (not in the commit message or the diff title).

Cheers,

Build has FAILED

Patch application report for D7859 (id=28373)

Rebasing onto b548d2f261...

Current branch diff-target is up to date.
Changes applied before test
commit 6404f5784608a304d0f53677122aac458398c117
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Thu May 19 09:20:36 2022 +0200

    fix mailto econding for inboud email

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1852/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1852/console

Build has FAILED

Patch application report for D7859 (id=28379)

Rebasing onto 12661f9b3f...

First, rewinding head to replay your work on top of it...
Applying: fix mailto econding for inboud email
Changes applied before test
commit 8905966ed768b66beb0caa8f45a8e2c69f89d8b4
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Thu May 19 09:20:36 2022 +0200

    fix mailto econding for inboud email

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1853/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1853/console

URL-encording done directly on the url construction of the mailto

Related to T4260

bchauvet retitled this revision from fix mailto econding for inboud email (T4260) to fix mailto encoding for inbound email (T4260).May 19 2022, 11:44 AM
assets/src/bundles/add_forge/request-dashboard.js
136

now the code escapes newlines in emailText twice. Replace .replace(/\n/g, '%0D%0A') with .replace(/\n/g, '\r\n'), and encodeURIComponent will then turn '\r\n' into '%0D%0A'.

(And we should probably have tests to catch this)

Build is green

Patch application report for D7859 (id=28383)

Rebasing onto 12661f9b3f...

First, rewinding head to replay your work on top of it...
Applying: fix mailto encoding for inbound email
Changes applied before test
commit 64a05da18a3dbbe18cbd99246ea2e0b5c7bfb4b0
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Thu May 19 09:20:36 2022 +0200

    fix mailto encoding for inbound email

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1855/ for more details.

vlorentz retitled this revision from fix mailto encoding for inbound email (T4260) to fix mailto encoding for inbound email.May 19 2022, 12:10 PM
vlorentz edited the summary of this revision. (Show Details)

Build is green

Patch application report for D7859 (id=28392)

Rebasing onto 12661f9b3f...

First, rewinding head to replay your work on top of it...
Applying: fix mailto encoding for inbound email
Changes applied before test
commit 78020b90a34d52ec4f57c357a693457bd1591af2
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Thu May 19 09:20:36 2022 +0200

    fix mailto encoding for inbound email

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1856/ for more details.

@bchauvet you pushed 7721b9b6a7a1 to master, but this diff's commit was fba125818391; so you need to close it manually

This revision is now accepted and ready to land.May 24 2022, 11:51 AM