Closed Bug 950105 Opened 7 years ago Closed 7 years ago
[Wap push] SI messages which contain '&' symbol in the URL are not received
+++ This bug was initially created as a clone of Bug #944422 +++ While investigating bug 944422 I found an issue in how WBXML code is parsed and decoded into a regular XML file. The WAP-192-WBXML-20010725-a standard  mandates that during encoding "character entities in the textual markup (e.g., &) which can be represented in the target character encoding MUST be converted to string form". In practice this means that an '&' sequence in the original markup will be turned into the '&' character in the WBXML message. Once the WBXML message is parsed and turned again into XML this process must be reversed for text fields and the following characters need to be turned into their character entity form: & -> & < -> < > -> > If we fail to do this the resulting XML document will be invalid as &, < and > are not permitted to appear unescaped in a text field. To fix this problem I believe we should detect and replace these characters with character entities in both WbxmlInlineString.decode()  and WbxmlStringTable.decode() . The relevant unit-tests in test_si_pdu_helper.js and test_sl_pdu_helper.js should also be augmented to cover these cases. This bug is currently causing WAP Push messages containing URL with search parameters (which thus include the '&' character) to be dropped by the WAP Push app as the DOMParser refuses the XML file delivered by gecko. I'm marking this bug as koi? since the original was a koi+. I'd tackle this bug myself but I spent most of today trying to run the xpcshell test within the emulator with no success so I'm not sure how long it could take me. Maybe someone more familiar with this code can take over.  http://technical.openmobilealliance.org/tech/affiliates/wap/wap-192-wbxml-20010725-a.pdf  http://hg.mozilla.org/mozilla-central/file/8b5875dc7e31/dom/wappush/src/gonk/WbxmlPduHelper.jsm#l137  http://hg.mozilla.org/mozilla-central/file/8b5875dc7e31/dom/wappush/src/gonk/WbxmlPduHelper.jsm#l119
Triage Notes: Sounds like a blocker. Search parameters are a common use case for a URL, so this is could definitely happen.
This is a WIP patch that implements escaping reserved XML characters when parsing inline string and string table references. I'd like to add some tests to verify this works correctly but I've been unable to run the WAP Push xpcshell tests on my device or on the emulator. I've followed the instructions on MDN https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/XPCShell but couldn't figure out what to pass to the --testing-modules-dir and --manifest parameters. Chuck can you have a look at my patch and see if it looks reasonable? If you can also help me out with xpcshell tests I should be able to finish this one pretty soon.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Attachment #8347966 - Flags: feedback?(chulee)
Comment on attachment 8347966 [details] [diff] [review] [PATCH WIP] Escape reserved characters when converting a WBXML document to XML Review of attachment 8347966 [details] [diff] [review]: ----------------------------------------------------------------- Look good to me, and thanks for fixing this. xpcshell for WAP Push can't run in emulator is caused by bug 948376, and is fixed now. you can add test case to dom/wappush/tests/test_cp_pdu_helper.js, and use run test by following command after build emulator. ./test.sh xpcshell --test-path B2G_EMULATOR_PATH/objdir-gecko/dist/test-package-stage/xpcshell/tests/dom/wappush/tests/test_cp_pdu_helper.js
Attachment #8347966 - Flags: feedback?(chulee) → feedback+
(In reply to Chuck Lee [:chucklee] from comment #3) > xpcshell for WAP Push can't run in emulator is caused by bug 948376, and is > fixed now. > you can add test case to dom/wappush/tests/test_cp_pdu_helper.js, and use > run test by following command after build emulator. > > ./test.sh xpcshell --test-path > B2G_EMULATOR_PATH/objdir-gecko/dist/test-package-stage/xpcshell/tests/dom/ > wappush/tests/test_cp_pdu_helper.js Ah, great! Thanks!
Here's an update patch complete with an additional xpcshell test to exercise this functionality. Enpei can you check if this patch fixes the issue you tested in bug 944422? The patch I'm attaching applies cleanly to gecko in the 1.2 branch. The behavior should now be identical to Android: when the 'href' field points to an URL with the '&' symbol it will be properly translated and the resulting link in the WAP Push app should be shown and should be identical to what you see in Android.
Comment on attachment 8349469 [details] [diff] [review] [PATCH] Escape reserved characters when converting a WBXML document to XML Review of attachment 8349469 [details] [diff] [review]: ----------------------------------------------------------------- Look good to me, Thanks! But I am not a peer, add vicamo as reviewer.
Hi Gabriele, Verified on Buri with attached patch. Gaia c82c957e9d4078cd4043de6b7d21a2667a73adf7 Gecko 65dfa0844f0abbe4cfc90708f8620df4811afe6a BuildID 20131219140435 Version 26.0 ro.build.version.incremental=291 ro.build.date=Wed Dec 4 09:48:04 CST 2013 Nicely work and it will display exactly what user send no matter "&" or "&". Also verified other possible special characters(! * ' ( ) ; : @ & = + $ , / ? % # [ ]- _ . ~).
Comment on attachment 8349469 [details] [diff] [review] [PATCH] Escape reserved characters when converting a WBXML document to XML Review of attachment 8349469 [details] [diff] [review]: ----------------------------------------------------------------- r=chulee,me. Thank you :)
Attachment #8349469 - Flags: review?(vyang) → review+
Thanks Chuck & Vicamo for the review and big thanks to Enpei for her thorough testing! The green try-run is here: https://tbpl.mozilla.org/?tree=Try&rev=0272bad783dc so setting checkin-needed; this needs to land on master and on mozilla-b2g26_v1_2, the patch should apply cleanly everywhere. I've set the flags accordingly, once landed the issue should be fixed across all affected versions.
koi+. please uplift.
blocking-b2g: koi? → koi+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to Wes Kocher (:KWierso) from comment #12) > https://hg.mozilla.org/mozilla-central/rev/778e7a7b267e Hi Wes, could you also help to uplift to v1.2 due to this is a koi+ bug. Thanks.
Verified on Buri v1.2 and passed. Gaia 01e9da49be2cc4bc134eeefc434740d572ec2246 Gecko 252332f9d779541d82b87b9e17b57a4d5a978443 BuildID 20131223095701 Version 28.0a2
You need to log in before you can comment on or make changes to this bug.