Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when opening server configuration for accounts imported from DC desktop #3408

Closed
anedroid opened this issue Nov 4, 2024 · 8 comments · Fixed by #3410
Closed

Crash when opening server configuration for accounts imported from DC desktop #3408

anedroid opened this issue Nov 4, 2024 · 8 comments · Fixed by #3410
Labels

Comments

@anedroid
Copy link

anedroid commented Nov 4, 2024

  • Android version: 14 (LineageOS FOSS)
  • Device: moto g7 power
  • Delta Chat version: 1.46.14 F-Droid
  • Delta Chat version: 1.46.8 Flathub
  • Steps to reproduce the problem:
    • Export chats from DC desktop
    • Move backup file to Android device
    • Add new account and import backup
    • Open settings/advanced/server configuration
  • Additional notes that might be needed to reproduce:
    • Server accessed through VPN or proxy
    • Local domain and self-signed certificate
    • It works and does not crash if you log in, then open server configuration
    • Didn't try with chatmail/public mail server yet
  • Logs:
AndroidRuntime: FATAL EXCEPTION: main
AndroidRuntime: Process: com.b44t.messenger, PID: 19651
AndroidRuntime: java.lang.ArrayIndexOutOfBoundsException: length=3; index=3
AndroidRuntime:         at java.util.Arrays$ArrayList.get(Arrays.java:4245)
AndroidRuntime:         at android.widget.ArrayAdapter.getItem(ArrayAdapter.java:394)
AndroidRuntime:         at android.widget.ArrayAdapter.createViewFromResource(ArrayAdapter.java:450)
AndroidRuntime:         at android.widget.ArrayAdapter.getView(ArrayAdapter.java:416)
AndroidRuntime:         at android.widget.Spinner.makeView(Spinner.java:729)
AndroidRuntime:         at android.widget.Spinner.layout(Spinner.java:677)
AndroidRuntime:         at android.widget.Spinner.onLayout(Spinner.java:639)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at androidx.constraintlayout.widget.ConstraintLayout.onLayout(ConstraintLayout.java:1873)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
AndroidRuntime:         at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
AndroidRuntime:         at android.widget.ScrollView.onLayout(ScrollView.java:1811)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
AndroidRuntime:         at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at androidx.appcompat.widget.ActionBarOverlayLayout.onLayout(ActionBarOverlayLayout.java:553)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
AndroidRuntime:         at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1891)
AndroidRuntime:         at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1729)
AndroidRuntime:         at android.widget.LinearLayout.onLayout(LinearLayout.java:1638)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
AndroidRuntime:         at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
AndroidRuntime:         at com.android.internal.policy.DecorView.onLayout(DecorView.java:807)
AndroidRuntime:         at android.view.View.layout(View.java:25159)
AndroidRuntime:         at android.view.ViewGroup.layout(ViewGroup.java:6460)
AndroidRuntime:         at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:4562)
AndroidRuntime:         at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3830)
AndroidRuntime:         at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2718)
AndroidRuntime:         at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:9937)
AndroidRuntime:         at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1406)
AndroidRuntime:         at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1415)
AndroidRuntime:         at android.view.Choreographer.doCallbacks(Choreographer.java:1015)
AndroidRuntime:         at android.view.Choreographer.doFrame(Choreographer.java:945)
AndroidRuntime:         at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1389)
AndroidRuntime:         at android.os.Handler.handleCallback(Handler.java:959)
AndroidRuntime:         at android.os.Handler.dispatchMessage(Handler.java:100)
AndroidRuntime:         at android.os.Looper.loopOnce(Looper.java:232)
AndroidRuntime:         at android.os.Looper.loop(Looper.java:317)
AndroidRuntime:         at android.app.ActivityThread.main(ActivityThread.java:8592)
AndroidRuntime:         at java.lang.reflect.Method.invoke(Native Method)
AndroidRuntime:         at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
AndroidRuntime:         at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878)
Process : Sending signal. PID: 19651 SIG: 9
.b44t.messenger: Using CollectorTypeCC GC.
.b44t.messenger: Not starting debugger since process cannot load the jdwp agent.
MultiDex: VM with version 2.1.0 has multidex support
MultiDex: Installing application
MultiDex: VM has multidex support, MultiDex support library is disabled.
DeltaChat: ++++++++++++++++++ ApplicationContext.onCreate() ++++++++++++++++++
@anedroid anedroid added the bug label Nov 4, 2024
@Hocuri
Copy link
Collaborator

Hocuri commented Nov 4, 2024

  • Open settings/advanced/server configuration

On my phone, there is only "Settings / Advanced / Password and Account", is this what you mean?

@Hocuri
Copy link
Collaborator

Hocuri commented Nov 4, 2024

The stack trace looks like one of the spinners (imap_security, smtp_security, auth_method, or cert_check) was set to an invalid value. Maybe cert_check, because it's the only one with exactly 3 possible values. What I don't understand is why there seems to be a fourth possible value on DC Desktop.

On the DC Desktop from where you exported your chats. Can you go to "Help / About Delta Chat" and tell the value of "imap certificate checks"? Or alternatively, in "Help / About Delta Chat", scroll down to "Copy JSON", paste the result here, and remove personal information before you post it here on GitHub (esp. it contains your email address)

@r10s
Copy link
Member

r10s commented Nov 4, 2024

yip, it is cert_check.

the crash is repproducible by changing RegistrationActivitiy.java, line228 to int certCheckFlags = 3;.

for fixing we need to make sure, the read config are inside the range 0..2 (or, better, real maximal index), defaulting to 0 if out of range.

wondering, why recent desktop sets that to 3 - that might be another issue, maybe some mixing up with smtp_security, which allows values up to 3. or were there recent changes in that area?

(came to that while searching if there was a PR for fixing that, i am not on a fix currently :)

@Hocuri
Copy link
Collaborator

Hocuri commented Nov 4, 2024

Desktop is right to set it to 3 - excerpt from deltachat.h:

/**
 * @defgroup DC_CERTCK DC_CERTCK
 *
 * These constants configure TLS certificate checks for IMAP and SMTP connections.
 *
 * These constants are set via dc_set_config()
 * using keys "imap_certificate_checks" and "smtp_certificate_checks".
 *
 * @addtogroup DC_CERTCK
 * @{
 */

/**
 * Configure certificate checks automatically.
 */
#define DC_CERTCK_AUTO 0

/**
 * Strictly check TLS certificates;
 * requires that both the certificate and the hostname are valid.
 */
#define DC_CERTCK_STRICT 1

/**
 * Accept invalid certificates, including self-signed ones
 * or having incorrect hostname.
 */
#define DC_CERTCK_ACCEPT_INVALID_CERTIFICATES 3

0, 1, and 3 are valid.

DC Android tries to have the correct behavior here in arrays.xml:

    <string-array name="pref_dc_certck_entries">
        <item>@string/automatic</item>
        <item>@string/strict</item>
        <item>@string/accept_invalid_certificates</item>
    </string-array>

    <string-array name="pref_dc_certck_values">
        <item>0</item>
        <item>1</item>
        <item>3</item>
    </string-array>

which is used from registration_activity.xml:

        <Spinner
[...]
            android:entries="@array/pref_dc_certck_entries"
            android:entryValues="@array/pref_dc_certck_values"

but for some reason this doesn't work.

I'll try to make a fix that simply doesn't use entryValues.

@Hocuri
Copy link
Collaborator

Hocuri commented Nov 4, 2024

Now I'm confused. The rust code says that both 2 and 3 are OK, but 3 is deprecated?

    /// Accept certificates that are expired, self-signed
    /// or otherwise not valid for the server hostname.
    AcceptInvalidCertificates = 2,

    /// Alias for `AcceptInvalidCertificates`
    /// for API compatibility.
    AcceptInvalidCertificates2 = 3,

https://github.com/deltachat/deltachat-core-rust/blob/37ca9d7319dabcf1cd3cf0bb75c2b4c062aa3bf0/src/login_param.rs#L18-L41

Edit: Core doesn't include any code to make all 2s into 3s, so I guess right now all UIs need to handle both 2 and 3?

@r10s
Copy link
Member

r10s commented Nov 4, 2024

is 3 really regarded as deprecated? imu it is treated equally to 2 in core - otherwise, we need to adapt cffi and at least iOS that writes a 3 as well (but accepts, different to android, also other values gracefully)

@Hocuri
Copy link
Collaborator

Hocuri commented Nov 4, 2024

is 3 really regarded as deprecated?

To me, the comment "Alias for AcceptInvalidCertificates for API compatibility." implies that it's deprecated, but if all UIs write 3, we can just adapt this comment not to make anyone think that it's deprecated and extend #3410 to write 3 instead of 2

imu it is treated equally to 2 in core

Yes, that's correct.

@adbenitez
Copy link
Member

  /// Alias for `AcceptInvalidCertificates`
    /// for API compatibility.
    AcceptInvalidCertificates2 = 3,

this seems to be deprecated, if it is only an alias for the real value, 2
and UIs should not be writing 3 to the database, but ofc accept to load 3 from old db, but always save as 2

r10s added a commit to deltachat/deltachat-core-rust that referenced this issue Nov 13, 2024
this PR changes `DC_CERTCK_ACCEPT_*` to the same values in cffi as rust
does. and regards the same values as deprecated afterwards

there is some confusion about what is deprecated and what not, see
deltachat/deltachat-android#3408

iOS needs to be adapted as it was following the docs in the CFFI before,
same desktop. both need to be graceful on reading and strict on writing.

~~**this PR is considered harmful,** so we should not merge that in
during 1.48 release, there is no urgency, things are fine (wondering if
it isn't even worth the effort, however, having different values and
deprecations is a call for trouble in the future ...)~~

---------

Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants