Skip to content
This repository has been archived by the owner on Jan 25, 2018. It is now read-only.

my final v14.1 work #634

Merged
merged 20 commits into from Feb 25, 2015
Merged

my final v14.1 work #634

merged 20 commits into from Feb 25, 2015

Conversation

eighthave
Copy link
Member

Here is my big wrap up for v14.1. It includes:

  • IOCipher v0.3
  • a prompt if the IOCipher media file or the SD card is missing
  • remove the "Keep/Delete" prompt after every voice message or embedded camera image. This made those features both really annoying to use since you got this extra prompt after sending every time. If the user wants to save the voice or image, they can export it from the chat log.
  • fixed auto-signin when there is more than one account setup
  • auto-resizing images taken with embedded camera, based on @n8fr8 's work in Feature downsize images #633
  • a couple odds and ends

v0.3 includes:

* improved mount error handling
* build and support fixes for 5.0/Lollipop/android-21

refs #4531 https://dev.guardianproject.info/issues/4531
With IOCipher v0.3, you have to explicitly create a new VFS file, whereas
before VirtualFileSystem.mount() would do it automatically.  This made it
hard to debug situations where something went wrong with the file at the
path that was stored, like the perms changed, or the file was deleted by
another process.  This change makes it more like the UNIX mount command.

refs #4531 https://dev.guardianproject.info/issues/4531
…keys

Calling VirtualFileSystem.unmount() now zeroes out stored keys and passwords
in memory, so it should be called on Shutdown & Lock.
This provides a central place to define what happens when the user does not
want to provide a password.
ChatSecure currently does some custom transformations on the password that
the user supplies to unlock the encrypted storee.  This change just moves
all those transformations to a single method so we can keep track of them,
and to ensure they are always used.

refs #4531 https://dev.guardianproject.info/issues/4531
…user

Right now, the IOCipher container file that is the encrypted media store is
not stored in a stable path.  If the External Storage is present when
ChatSecure starts, then it is used, otherwise the Internal Storage is used.
But if ChatSecure starts, then the user ejects the External Storage,
ChatSecure will just try the External Storage path, then crash when it is
not there.  Or vice-versa: if ChatSecure is started when the External
Storage is not mounted, then the user mounts the External Storage,
ChatSecure will look for the media store on External Storage, and crash.

The first step to fixing this problem is storing a preference of where the
media store file should be, so ChatSecure can know whether to just create a
new media store because there never was one, or whether to prompt the user
to put the External Storage back in.  To do that, we have to detect all the
possible variations that could have happened, so there is some crazy logic
in IocVfs.init().
No need to try mounting the VFS again if it is already mounted.  Really,
the code should be refactored so that IocVfs.init() is never called more
than once, but that is a larger project.
… usage

AccountWizardActivity had accumulated more dead code than living, making it
hard to decipher how the whole "Shutdown & Lock" mechanism works.
This creates a single static method for calling "Shutdown & Lock", so that
other Activities can call it without recreating this code.  Specifically,
for the error dialog when the IOCipher media file is missing or the SD Card
is not present.

This probably should instead be implemented as a static method on ImApp,
but this would require changing how CacheWord works, and that's a bigger
project than should be tackled right before a release.

This follows up on the previous commit, which removed a dead version of
this same method.
…ivity

Since chat_screen_menu.xml is only used in NewChatActivity, it should have
a name that matches so it is easy to find which menu goes to which Activity.
The media store was sometimes stored on the External Storage, which can be
removed or even replaced with a different SD Card.  This adds a prompt
screen at startup to warn the user about this, and give them two options
for fixing the situation: delete references to the existing media store or
try changing SD cards.

closes #4531 https://dev.guardianproject.info/issues/4531
I could never remember what IocVfs was, or vice versa.  I think
ChatFileStore explains it well.
…space

There should be at least 512MB free on the internal storage, otherwise
prefer the external storage, if available.
…ally

This removes the Dialog that asks the user whether they want to delete the
original audio or picture file that they just took from ChatSecure in
order to send in a message.  There is no need for this Dialog and it is a
rather big annoyance in a "push to talk" session. If the user wants to
save an image or audio file, they'll first want to see or hear it, which
they can do once it is displayed in the message view.  If they want to
keep it, they can long-press it and choose "Export".
Based on the method names, it looks like this code structure is left over
from when things behaved quite a bit differently.  For example, launching
NewChatActivity is the sole action of a method called showAccounts(), and
showActiveAccount() iterates through all active accounts and launches a new
Intent to show NewChatActivity.
There was a bug on start up when there are multiple accounts set up.
Something is setting one of the accounts as already signed in, i.e.
Imps.ConnectionStatus.ONLINE before signInAll() is run.  It could also be that
those values were not updated when ChatSecure shutdown, or ChatSecure was
killed before it could update those values.

Really, the offline/connecting/online status info should be fetched live from
XMPP/SMACK rather than using some stored value in the database.  The only such
value that should be stored is ACTIVE_ACCOUNT_KEEP_SIGNED_IN to see whether
the user wants that account to start up automatically. Right now there are
four very similar values stored, but not really consistently used:

* Imps.Account.LAST_LOGIN_STATE
* Imps.Provider.ACCOUNT_CONNECTION_STATUS
* Imps.Provider.ACTIVE_ACCOUNT_KEEP_SIGNED_IN
* Imps.AccountStatus.CONNECTION_STATUS

The particular bit of code changed in this commit only seems to be used when
ChatSecure is setup with a password.  With ChatSecure is used without a
password, the startup stuff is handled in RemoteImService.autoLogin().
I got this when I ended a chat session that contained just the message from
Calyx that only OTR messages are allowed.

java.lang.NullPointerException
    at info.guardianproject.otr.app.im.app.ChatView.setSelected(ChatView.java:240)
    at info.guardianproject.otr.app.im.app.NewChatActivity$ChatViewFragment.onSelected(NewChatActivity.java:2069)
    at info.guardianproject.otr.app.im.app.NewChatActivity$4.onPageSelected(NewChatActivity.java:253)
    at android.support.v4.view.ViewPager.scrollToItem(ViewPager.java:572)
    at android.support.v4.view.ViewPager.setCurrentItemInternal(ViewPager.java:556)
    at android.support.v4.view.ViewPager.setCurrentItemInternal(ViewPager.java:514)
    at android.support.v4.view.ViewPager.setCurrentItem(ViewPager.java:495)
    at info.guardianproject.otr.app.im.app.NewChatActivity.showChat(NewChatActivity.java:732)
    at info.guardianproject.otr.app.im.app.NewChatActivity$5.onLoadFinished(NewChatActivity.java:344)
    at info.guardianproject.otr.app.im.app.NewChatActivity$5.onLoadFinished(NewChatActivity.java:1)
    at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(LoaderManager.java:427)
    at android.support.v4.app.LoaderManagerImpl.initLoader(LoaderManager.java:562)
    at info.guardianproject.otr.app.im.app.NewChatActivity.initChats(NewChatActivity.java:324)
    at info.guardianproject.otr.app.im.app.NewChatActivity.access$27(NewChatActivity.java:321)
    at info.guardianproject.otr.app.im.app.NewChatActivity$12.onLoadFinished(NewChatActivity.java:1730)
    at info.guardianproject.otr.app.im.app.NewChatActivity$12.onLoadFinished(NewChatActivity.java:1)
    at android.support.v4.app.LoaderManagerImpl$LoaderInfo.callOnLoadFinished(LoaderManager.java:427)
    at android.support.v4.app.LoaderManagerImpl$LoaderInfo.onLoadComplete(LoaderManager.java:395)
    at android.support.v4.content.Loader.deliverResult(Loader.java:104)
    at android.support.v4.content.CursorLoader.deliverResult(CursorLoader.java:73)
    at android.support.v4.content.CursorLoader.deliverResult(CursorLoader.java:35)
    at android.support.v4.content.AsyncTaskLoader.dispatchOnLoadComplete(AsyncTaskLoader.java:223)
    at android.support.v4.content.AsyncTaskLoader$LoadTask.onPostExecute(AsyncTaskLoader.java:61)
    at android.support.v4.content.ModernAsyncTask.finish(ModernAsyncTask.java:461)
    at android.support.v4.content.ModernAsyncTask.access$500(ModernAsyncTask.java:47)
    at android.support.v4.content.ModernAsyncTask$InternalHandler.handleMessage(ModernAsyncTask.java:474)
    at android.os.Handler.dispatchMessage(Handler.java:102)
    at android.os.Looper.loop(Looper.java:136)
    at android.app.ActivityThread.main(ActivityThread.java:5001)
    at java.lang.reflect.Method.invokeNative(Native Method)
    at java.lang.reflect.Method.invoke(Method.java:515)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
    at dalvik.system.NativeStart.main(Native Method)
               Process  I  Sending signal. PID: 23200 SIG: 9
Large files can take a long time to transfer over OTRDATA since many XMPP
servers will throttle the connection if too much data is sent at once.
Since images taken with the embedded camera are more ephemeral in nature,
they do not need to be high quality most of the time.  So this resizing
those images before they are stored in the ChatFileStore and sent using
OTRDATA.

This will also help with devices with limited storage, especially on the
internal storage, since it will keep the IOCipher container file smaller.

replaces guardianproject#633
@n8fr8
Copy link
Member

n8fr8 commented Feb 25, 2015

So cee8a3a makes my PR moot? I should cancel it?

Otherwise, wooooo! 👍


SharedPreferences settings = PreferenceManager.getDefaultSharedPreferences(context);
if (settings.getBoolean(
context.getString(R.string.key_store_media_on_external_storage_pref), false))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is false by default, then the first time this pref is set (i.e. on an upgrade where the 'key_store_media_on_external_storage_pref' didn't exist before) would be false, and switch to the internal dbFilePath, which would then be created and mounted.

I think if the pref doesn't exist, we should check if the external path exists first, and then set to true, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chances of that line of code returning the default value should be none. The preference should be guarateed to be set before ChatFileStore.init() is ever run because of the checks that happen in WelcomeActivity. There, it sets the default based on whether it detected media store files on internal and/or external storage:
eighthave@9c99e50

These make it really easy to set up bridges in Orbot.
@eighthave
Copy link
Member Author

I just added a small commit to make bridge: links always clickable, whether ChatSecure is using Tor or not. They should be safe links always, right?

n8fr8 added a commit that referenced this pull request Feb 25, 2015
@n8fr8 n8fr8 merged commit 25de18f into guardianproject:master Feb 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants