creswick / StreamingQR

Apache License 2.0
2 stars 0 forks source link

NPE during TransmitFragment.sendJob() on Nexus 4 #58

Closed creswick closed 10 years ago

creswick commented 10 years ago

I ran into this with some file transfers initiated on my Nexus 4, using branch #38. Creating a new ticket because I don't think it's related to the changes in #38, although I have not tested on Master.

Reproduction:

E/AndroidRuntime( 4166): java.lang.NullPointerException
E/AndroidRuntime( 4166):    at com.galois.qrstream.lib.TransmitFragment.transmitData(TransmitFragment.java:147)
E/AndroidRuntime( 4166):    at com.galois.qrstream.lib.TransmitFragment.sendJob(TransmitFragment.java:126)
E/AndroidRuntime( 4166):    at com.galois.qrstream.lib.TransmitFragment.access$100(TransmitFragment.java:33)
E/AndroidRuntime( 4166):    at com.galois.qrstream.lib.TransmitFragment$3.onLayoutChange(TransmitFragment.java:111)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14826)
E/AndroidRuntime( 4166):    at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1671)
E/AndroidRuntime( 4166):    at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1525)
E/AndroidRuntime( 4166):    at android.widget.LinearLayout.onLayout(LinearLayout.java:1434)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14817)
E/AndroidRuntime( 4166):    at android.view.ViewGroup.layout(ViewGroup.java:4631)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14817)
E/AndroidRuntime( 4166):    at android.view.ViewGroup.layout(ViewGroup.java:4631)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14817)
E/AndroidRuntime( 4166):    at android.view.ViewGroup.layout(ViewGroup.java:4631)
E/AndroidRuntime( 4166):    at com.android.internal.widget.ActionBarOverlayLayout.onLayout(ActionBarOverlayLayout.java:374)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14817)
E/AndroidRuntime( 4166):    at android.view.ViewGroup.layout(ViewGroup.java:4631)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
E/AndroidRuntime( 4166):    at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
E/AndroidRuntime( 4166):    at android.view.View.layout(View.java:14817)
E/AndroidRuntime( 4166):    at android.view.ViewGroup.layout(ViewGroup.java:4631)
E/AndroidRuntime( 4166):    at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:1987)
E/AndroidRuntime( 4166):    at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1744)
E/AndroidRuntime( 4166):    at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1000)
E/AndroidRuntime( 4166):    at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5670)
E/AndroidRuntime( 4166):    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:761)
E/AndroidRuntime( 4166):    at android.view.Choreographer.doCallbacks(Choreographer.java:574)
E/AndroidRuntime( 4166):    at android.view.Choreographer.doFrame(Choreographer.java:544)
E/AndroidRuntime( 4166):    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:747)
E/AndroidRuntime( 4166):    at android.os.Handler.handleCallback(Handler.java:733)
E/AndroidRuntime( 4166):    at android.os.Handler.dispatchMessage(Handler.java:95)
E/AndroidRuntime( 4166):    at android.os.Looper.loop(Looper.java:136)
E/AndroidRuntime( 4166):    at android.app.ActivityThread.main(ActivityThread.java:5017)
E/AndroidRuntime( 4166):    at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime( 4166):    at java.lang.reflect.Method.invoke(Method.java:515)
E/AndroidRuntime( 4166):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779)
E/AndroidRuntime( 4166):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595)
E/AndroidRuntime( 4166):    at dalvik.system.NativeStart.main(Native Method)
creswick commented 10 years ago

add more reproduction details

donpdonp commented 10 years ago

I tried sharing a photo, the transmission started w/o errors. Sharing a text file or photo from Andro File Manager did not list QRStream as an option :(.

creswick commented 10 years ago

This NPE is happening in TransmitFragment.transmitData(Job job), in the log message:

Log.i(Constants.APP_TAG, "transmitData title=" + title + " byte count=" + bytes.length);

bytes is apparently null, so the Job is being created with a null parameter (which should probably never happen). Digging into that, this appears to happen every time a file:/// uri is sent (or more generally, any time a dataUrl is shared that does /not/ have the "content" scheme.)

This should crash for any file-manager generated share action.

We should at least set some defensive practices in place:

Any file share from Astro will trigger this, because it sends file:/// urls, but we can't assume that that's the only other url scheme that will show up -- we should be handling them more robustly.

Reproduction:

QRstream should crash with a NPE in com.galois.qrstream.lib.TransmitFragment.transmitData(TransmitFragment.java:163) (using master as of the current date).

donpdonp commented 10 years ago

Added a check for null data in Job, throws Invalid parameter exception. Added file:// URI, works the same as content://.

creswick commented 10 years ago

Could you add a debug log to the (missing) else branch in MainActivity line 204? I could see us being stumped later, and wondering why something is falling through these conditionals -- a log statement will help at runtime and serve as documentation in the source.

Other than that, looks good to merge.

if(dataUrl != null) {
            name = getNameFromURI(dataUrl);
            if (dataUrl.getScheme().equals("content") ||
                dataUrl.getScheme().equals("file")) {
                try {
                    bytes = readFileUri(dataUrl);
                } catch (IOException e) {
                    e.printStackTrace();
                }
            } else {
                 // log here.
            }
        } else {
            // fall back to content in extras (mime type dependent)
            if(type.equals("text/plain")) {
                String body = extras.getString(Intent.EXTRA_SUBJECT) +
                              extras.getString(Intent.EXTRA_TEXT);
                bytes = body.getBytes();
            }
        }
donpdonp commented 10 years ago

log statement added, and branch merged with master.