Closed
Bug 1006781
Opened 10 years ago
Closed 10 years ago
[B2G][MMS] No thumbnail preview when sending or recieving an MMS when CSP 1.0 parser is enabled
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(b2g-v1.4 unaffected, b2g-v2.0 affected)
RESOLVED
DUPLICATE
of bug 1012663
Tracking | Status | |
---|---|---|
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | affected |
People
(Reporter: jschmitt, Unassigned)
References
Details
(Whiteboard: [sms-most-wanted])
Attachments
(2 files)
Description: Thumbnail images are missing when sending an image through the messaging app. Repro Steps: 1) Update a Open_C to BuildID: 20140506040204 2) Open the Messaging app 3) Create a new message 4) Input a valid phone number 5) Attach an image from either Camera or Gallery 6) Notice no preview when sending the image Actual: There is no thumbnail preview when sending an image mms. Expected: Thumbnail images are shown when sending an mms. 2.0 Environmental Variables: Device: Open_C 2.0 BuildID: 20140506040204 Gaia: 4af716f09747edfbea637f5b60f7fd7d0183f19b Gecko: 81651ad5e43c Version: 32.0a1 Firmware Version: FFOS_US_EBAY_P821A10V1.0.0B06_LOG_DL Notes: Repro frequency: 100% See attached: logcat
Reporter | ||
Comment 1•10 years ago
|
||
Issue repros on 2.0 Buri build. Issue does not repro on 1.4 Open_C build. 1.4 Environmental Variables: Device: Open_C 1.4 BuildID: 20140506000202 Gaia: b1242f33981024de59b8b4c26bacff8b876211b1 Gecko: fe4080728c60 Version: 30.0 Firmware Version: P821A10-ENG_20140410
status-b2g-v1.4:
--- → unaffected
Keywords: regression,
regressionwindow-wanted
Updated•10 years ago
|
QA Contact: pcheng
Comment 2•10 years ago
|
||
Regression window was done using Buri on master builds. Mozilla inbound Regression Window: Last Working Environmental Variables: Device: Buri MOZ BuildID: 20140505103237 Gaia: e8a08a3f7a608993f0b302371e016e73faceea70 Gecko: 81587bb1f916 Version: 32.0a1 Firmware Version: v1.2-device.cfg First Broken Environmental Variables: Device: Buri MOZ BuildID: 20140505111137 Gaia: e8a08a3f7a608993f0b302371e016e73faceea70 Gecko: 7b05ebf0a2d5 Version: 32.0a1 Firmware Version: v1.2-device.cfg Gaia is the same on both builds, indicating that this is a Gecko issue. Mozilla inbound pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=81587bb1f916&tochange=7b05ebf0a2d5
Keywords: regressionwindow-wanted
Comment 3•10 years ago
|
||
Maybe caused by bug 858787? Garrett - What do you think?
Flags: needinfo?(grobinson)
Comment 5•10 years ago
|
||
I haven't tried it myself yet, is the preview missing in the composer while composing the message, or in the thread after sending the message, or both ? If it's only in the composer (where we're using an iframe), then it could definitely be a CSP issue. And actually, these messages are the proof: 05-06 13:29:02.723: E/GeckoConsole(2304): [JavaScript Warning: "Content Security Policy: The page's settings blocked the loading of a resource: An attempt to apply inline style sheets has been blocked" {file: "app://sms.gaiamobile.org/index.html#thread=4" line: 0 column: 0 source: "background-image: url(data:image/jpeg;ba..."}] 05-06 13:29:02.723: E/GeckoConsole(2304): [JavaScript Warning: "Content Security Policy: The page's settings blocked the loading of a resource: An attempt to apply inline style sheets has been blocked" {file: "app://sms.gaiamobile.org/index.html#thread=4" line: 0}] 05-06 13:29:03.123: D/NetlinkEvent(277): Unexpected netlink message. type=0x0 I don't know what we should do here; should we relax the CSP checks for the app? Is there an issue in the CSP check? Or should we find a way that does not fail with CSP? Also, the patch from bug 995116 will change this (move from data/url to a blob/url) so maybe this bug will be resolved (althouth I doubt it). Is it possible to test with the patch from bug 995116 ?
Flags: needinfo?(felash)
Keywords: qawanted
Comment 6•10 years ago
|
||
Given that the potential cause has been backed out, we should retest this on today's build to see if this still reproduces.
Comment 7•10 years ago
|
||
jsmith, that was very likely caused by the patches from bug 858787 (they have since been backed out), per the logging in comment 5. Note that the default certified app CSP *blocks* inline style, and always has [0] No certified app should use inline style and expect to work. As noted in bug 968906 (which will be resolved by bug 858787), this policy was not being fully enforced and so inline styles were incorrectly being allowed. We have bug 968907 to track removing inline styles from all of the default certified apps that we ship. [0] https://developer.mozilla.org/en-US/Apps/CSP
Flags: needinfo?(grobinson)
Comment 9•10 years ago
|
||
As Garrett pointed out correctly, the default CSP policy for certified apps does not allow any inline styles [1]. We should not mark this as fixed 'by backout' but rather fix the problem of using inline styles for certified apps. As soon as 951457 lands, we will only have a parser that enforces spec compliant policies, which means CSP will block all inline styles for certified apps. Julien, is there an easy way/fix to load that image from an external resource? [1] http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#397
Flags: needinfo?(felash)
Comment 10•10 years ago
|
||
Talked with Christoph in person - he would like to keep this bug open to get the CSP issue resolved so that we can land CSP support without smoketest regressions.
blocking-b2g: 2.0+ → ---
Keywords: regression,
smoketest
Summary: [B2G][MMS] No thumbnail preview when sending or recieving an MMS → [B2G][MMS] No thumbnail preview when sending or recieving an MMS when CSP 1.0 parser is enabled
Comment 11•10 years ago
|
||
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #9) > As Garrett pointed out correctly, the default CSP policy for certified apps > does not allow any inline styles [1]. We should not mark this as fixed 'by > backout' but rather fix the problem of using inline styles for certified > apps. As soon as 951457 lands, we will only have a parser that enforces spec > compliant policies, which means CSP will block all inline styles for > certified apps. > > Julien, is there an easy way/fix to load that image from an external > resource? We're generating an iframe content here, and the image itself is dynamic, so it's really not ideal nor easy. Is it possible to create an iframe with relaxed CSP rules? What's the problem with inline styles exactly (apart from bad practice in general)? I can only think of dirty things like generating a CSS "file" from a data or blob URL, but that does not really satisfy me. FTR, we use a background image because we want to leverage the fact that background-size keeps the width/height ratio.
Flags: needinfo?(felash)
Comment 12•10 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #6) > Given that the potential cause has been backed out, we should retest this on > today's build to see if this still reproduces. if it's still needed to confirm, the issue no longer reproduces on today's trunk build Open C v2.0 BuildID: 20140507040203 Gaia: 870a5c518742665d36b17e7e88c2ab07d440b94c Gecko: 417acde736e7 Version: 32.0a1 P821A10V1.0.0B06_LOG_DL
Comment 13•10 years ago
|
||
> Is it possible to create an iframe with relaxed CSP rules? Hm, if the inline styles are only in the iframe, than CSP should not be interfering (CSP is not inherited by a protected document's frames). This might be a little different on B2G, though. Maybe a certified app's iframes also have APP_STATUS_CERTIFIED? A quick way to test this would be to add the mozbrowser attribute to the attachment thumbnail iframe, since it short-circuits the GetAppStatus call in nsDocument::InitCSP and prevents the default certified app CSP from loading. Otherwise, this might be a bug. > What's the problem with inline styles exactly (apart from bad practice in general)? It enables an attacker who can inject style to perform window redressing aka phishing attacks. This is why it is blocked as part of the default certified app CSP. > I can only think of dirty things like generating a CSS "file" from a data or blob URL, but that does not really satisfy me. I'd want to avoid this workaround too. You will always have to change some of your development practices to conform with CSP, but I'd like to avoid making it onerous unless necessary.
Comment 14•10 years ago
|
||
I'll need to check where the inline style exactly is. How urgent is it? Also, pardon this sort of dumb question, what do we call exactly 'inline style' in the CSP context? 1. Is it an attribute "style="? 2. Is it a "<style>" tag? 3. Is it specifying "element.style.backgroundImage = ..." in JavaScript? 4. What about "element.style = "background-image: ..." ? 5. What about "element.setAttribute('style', 'background-image: ...')" ?
Comment 15•10 years ago
|
||
julienw, 1+2 are what is meant by "inline style". 3-5 are CSSOM setters, and *are not* be blocked by CSP (I just double-checked). There was a proposal for CSP to control CSSOM (via style-src 'unsafe-eval', not 'unsafe-inline') [0] but that has stalled out in the WG. [0] https://bugzilla.mozilla.org/show_bug.cgi?id=873302
Comment 16•10 years ago
|
||
Can you please cc me in bug 968906?
Comment 17•10 years ago
|
||
Julien, if you tell me how I can reproduce this problem on my local machine, I can help you fix the problem with the inline style blocking? I do have an emulator setup, is that sufficient to run smoketests, or this particular test?
Comment 18•10 years ago
|
||
Minor correction to Comment 15: 5 is not a CSSOM setter, you are setting the style attribute on an element, and that will be blocked by CSP as "inline style". It's basically the same case as 1, only set by JS. So, to be clear: 1,2,5 are blocked by CSP as "inline style" 3,4 are not (CSSOM)
Comment 19•10 years ago
|
||
Random hunch from skimming the code. It looks like you use an iframe for draft attachments, but a div for non-draft attachments [0]. An iframe should not have the CSP of the parent document automatically applied/inherited - if it does, that's a bug. However, a div is of course subject to the parent docuemnt's CSP. Perhaps that is what's causing the problem here? If you find that the CSP is being auto-applied to the iframe, then that is a bug we need to investigate. needinfo for this question, and also Comment 17. [0] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/attachment.js#L156
Flags: needinfo?(felash)
Comment 20•10 years ago
|
||
Comment 21•10 years ago
|
||
Yeah, I was actually building a current central right now, and enabling the pref, to reproduce. Because my question in comment 5 was never answered. The actual used markup is made from templates at [1]. So it seems that none of these cases are working, including the inline "font-size" in the iframe. Seems like that at least the "style=" markup used to inject the background-image will need to be changed to (probably) a JS-based code using the CSSOM. [1] https://github.com/mozilla-b2g/gaia/blob/014d16eaaf09fd9f0f57e5cac7aa2008dc764f2b/apps/sms/index.html#L472-L509 BTW, could the CSP being applied in an iframe also produce bug 963137 ? From my PoV actually it looks fine if the CSP applies to an iframe that has the same origin than the application.
Comment 22•10 years ago
|
||
Sorry, I somehow missed comment 17. You "only" need to send a MMS with an image to yourself. You don't even need to have network working, because when there is an error, the message is still added to the thread, so it should work on the emulator. There are 2 issues then: we don't see the image in the thread (it's in a <div> with an inline style attribute) nor in the "composer" part when you compose the message (it's in an <iframe> that contains also a markup with an inline style attribute -- same markup).
Flags: needinfo?(felash)
Comment 23•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #22) > Sorry, I somehow missed comment 17. > > You "only" need to send a MMS with an image to yourself. You don't even need > to have network working, because when there is an error, the message is > still added to the thread, so it should work on the emulator. Unfortunately it's not that simple. I can run the emulator, but there are no pre-existing picture available in the gallery and I can not take one myself because my desktop machine does not have a camera :-). I am pretty sure there is a way I can adb-push some picture into some folder, so I can access it in the emulator right? Maybe it's just easier to run the smoketest which causes that problem!
Flags: needinfo?(felash)
Comment 24•10 years ago
|
||
Holding the 'Home' button on my keyboard and pressing the 'power' button in the emulator allows you to take a screenshot, which I can then use to load into an MMS. Unfortunately the emulator gets stuck with the 'spinning wheel' right next to the image when attaching to an MMS. So I can't really tell if that image got loaded properly or not. What I can tell so far, there was no call to CSP that got blocked. Anyway, this needs further investigation. If I would have better explanation of the test-setup, that would simplify things a lot on my end.
Comment 25•10 years ago
|
||
You can try to push the reference workload using: APP=sms make reference-workload-light From the Gaia directory. This will push a small db of dummy messages containing MMS with images. From there, you can display the various threads (most of them contains MMS, there is one near the start called BIG-THREAD-MMS containing only MMS). If you longclick a message you can also forward it, and this way you can exercize the "compose" issue. If you can enable the CSP on Firefox, you can try to reproduce in Firefox too. Run: PROFILE_FOLDER=profile-sms DEBUG=1 DESKTOP=0 make /path/to/firefox --no-remote -profile ./profile-sms Then load "http://sms.gaiamobile.org:8080" in this firefox session, and press CTRL-SHIFT-M to use the "responsive mode" that gives you a mobile form factor. From there you can manipulate the SMS app with dummy data. The thread 052780 (the last one) contains MMS with images. You can't attach images in this environment but you can also forward a message using a right click on a message. Hope this helps reproducing on your side.
Flags: needinfo?(felash)
Comment 26•10 years ago
|
||
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #23) > (In reply to Julien Wajsberg [:julienw] from comment #22) > > Sorry, I somehow missed comment 17. > > > > You "only" need to send a MMS with an image to yourself. You don't even need > > to have network working, because when there is an error, the message is > > still added to the thread, so it should work on the emulator. > > Unfortunately it's not that simple. I can run the emulator, but there are no > pre-existing picture available in the gallery and I can not take one myself > because my desktop machine does not have a camera :-). I am pretty sure > there is a way I can adb-push some picture into some folder, so I can access > it in the emulator right? Maybe it's just easier to run the smoketest which > causes that problem! And you can probably push an image to /sdcard. This needs to be a small image (smaller than 5 Megapixels) to be picked up by the gallery.
Comment 27•10 years ago
|
||
julien, we dug into this today and verified that the iframe does indeed inherit the parent's CSP. This is intentional, because the iframe is created by document.createElement and doesn't have a src attribute. It gets a copy of the parent's principal, which includes its CSP [0]. See the discussion on seceng for more details. [0] http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsFrameLoader.cpp#518 Either way, the code needs to be changed to use the CSSOM, as you suggested in Comment 21. We will be sending a follow-up email to dev-gaia shortly, so everyone is reminded me of the default CSP policies. If you have any major problems conforming to the default policy please let us know.
Comment 28•10 years ago
|
||
Garett, I'd like to know when you'd need this change to be done. Especially, does it need to be done before the end of next week, or can it wait the week after? This is to plan our team's work :) Thanks !
Comment 29•10 years ago
|
||
Julien, we are going to start landing patches for the new CSP implementation this week which will co-exist with the current implementation and live behind a pref for some time. To answer your question, you don't have to fix the problem this week, but the sooner the better. When running the emulator with the new implementation (in other words blocking inline styles for certified apps) we realized that CSP is blocking several other inline styles (e.g. from the keyboard). We will reach out in an email to dev-gaia today and will make developers aware of the upcoming switch. We would like to do the switch as soon as possible, but also all of the discovered problems regarding inline style blocking in gaia needs to be rewritten first.
Comment 30•10 years ago
|
||
Ok thanks ! I'm setting a flag so that we won't forget the bug when we plan the next sprint.
blocking-b2g: --- → 2.0?
Comment 31•10 years ago
|
||
triage: according to comment 29, this needs to be fixed.
blocking-b2g: 2.0? → 2.0+
Comment 32•10 years ago
|
||
FYI we couldn't take this for the new sprint as we had feature work to finish. Please tell me if this is blocking for other work and I can try to reprioritize.
Comment 33•10 years ago
|
||
(In reply to Wesley Huang [:wesley_huang] from comment #31) > triage: according to comment 29, this needs to be fixed. This actually only needs to be fixed if the CSP 1.0 parser gets enabled, which is currently not the case. Given the current regression risk of that feature, we should push that into 2.1 at this point to decrease the # of regressions possible. Not a blocker unless CSP 1.0 gets enabled.
blocking-b2g: 2.0+ → ---
Comment 34•10 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #33) > (In reply to Wesley Huang [:wesley_huang] from comment #31) > > triage: according to comment 29, this needs to be fixed. > > This actually only needs to be fixed if the CSP 1.0 parser gets enabled, > which is currently not the case. Given the current regression risk of that > feature, we should push that into 2.1 at this point to decrease the # of > regressions possible. > > Not a blocker unless CSP 1.0 gets enabled. Jason, Julien, just as an update: bug 951457 landed a few weeks ago; currently the new csp-parser lives behind a pref (security.csp.newbackend.enable) which we are going to flip for FF32. The new CSP parser only supports CSP 1.0+ and therefore will cause this to break. When do you think, you can have that part updated? If I remember correctly, this was the only smoke test that failed when enabling the spec-compliant parser, right?
Flags: needinfo?(jsmith)
Flags: needinfo?(felash)
Comment 35•10 years ago
|
||
Can we followup offline on this with bbajaj? From the conversation I had with her on this, we wanted to have this wait until FF33 to avoid destabilizing 2.0 at this point, as we've already got too many regressions in the 2.0 release as is.
Flags: needinfo?(jsmith)
Comment 36•10 years ago
|
||
Are there other major bugs caused by CSP landing? If this was identified 3 weeks ago seems like we should have a good idea of the risk by now.
Comment 37•10 years ago
|
||
(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #36) > Are there other major bugs caused by CSP landing? If this was identified 3 > weeks ago seems like we should have a good idea of the risk by now. See the dependencies on bug 858787. Note - this was preffed on for only a day when we were only doing 2.0 smoketesting, so we don't understand the full breadth of the regression risk right now. We instead only know of an initial set of fallouts that occurred when we flipped the switch for a day's period of time.
Comment 38•10 years ago
|
||
I'd like to have this as part of next sprint so that we can flip the pref as early as possible in the 2.1 cycle.
Flags: needinfo?(felash)
Whiteboard: [sms-most-wanted]
Comment 39•10 years ago
|
||
That would mean a fix by June 20th at the latest.
Updated•10 years ago
|
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•