fix: Refactor AnkiServer for future HTTPS support (Issue #15991) by mixelas · Pull Request #20885 · ankidroid/Anki-Android

mixelas · 2026-04-28T19:25:35Z

Purpose / Description

This PR prepares the codebase for HTTPS support by refactoring AnkiServer to support SSL/TLS configuration, while maintaining backward compatibility.

Fixes

Fixes [BUG]: Restricting cleartext network traffic breaks import and statistics pages #15991

Approach

Added Context parameter to AnkiServer constructor to enable SSL setup when needed

Created SslUtil as a placeholder for future certificate generation
Updated all AnkiServer instantiations to pass context where available
Added unit tests for AnkiServer and SslUtil to verify basic functionality
Maintained full backward compatibility

How Has This Been Tested?

Full gradle build was successful
Kotlin compilation was successful
Unit tests created for:
AnkiServer baseUrl() returning HTTP when context not provided
AnkiServer baseUrl() containing localhost address
SslUtil placeholder initialization

Tested on:

Android Gradle Plugin 9.3.1
Kotlin 1.9.x
Java 21

Learning (optional, can help others)

While i was working with Android SDK, i noticed that it doesn't include sun.security internal APIs in modern versions. The proper approach for self-signed certificate generation on Android involves either:

Using BouncyCastle library
Using Android's KeyStore and KeyGenParameterSpec APIs

Checklist

Please, go through these checks before submitting the PR.

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

)

david-allison

Are you planning on adding an implementation? Otherwise this PR doesn't feel mergeable on its own

BrayanDSO

Have you used AI on this PR?

BrayanDSO · 2026-04-29T15:35:53Z

@@ -0,0 +1,78 @@
+/*
+ *  Copyright (c) 2024 AnkiDroid Contributors


copyright is wrong

I just copied this from another file in the codebase. I wanted it to look like the other files in there. I also didn't add my name because i didn't think that this would get merged.

BrayanDSO · 2026-04-29T15:36:19Z

+    private const val KEYSTORE_FILENAME = "anki_keystore.bks"
+    private const val KEYSTORE_PASSWORD = "ankidroid"
+    private const val KEY_PASSWORD = "ankidroid"


I guess that hardcoding these values isn't safe

Their purpose was strictly as a placeholder. I didn't think that you would entrust me to do the whole thing so i made this instead.

BrayanDSO · 2026-04-29T15:36:31Z

+
+        // TODO: Generate self-signed X.509 certificate
+        // Currently creates empty keystore. Future implementations should use:
+        // - Android KeyStore API (preferred), or


we probably should be doing that now

I acknowledge this is incomplete. I didn't know which architecture you preferred.

BrayanDSO · 2026-04-29T15:36:55Z

    val statesMutationEvalFlow = MutableSharedFlow<String>()

-    override val server: AnkiServer = AnkiServer(this, repository.getServerPort()).also { it.start() }
+    override val server: AnkiServer = AnkiServer(this, null, repository.getServerPort()).also { it.start() }


this means the issue still isn't fixed on the new study screen

I see the concern, if I passed null as the context, HTTPS wouldn't be enabled on the new study screen, which defeats the purpose of this PR.

BrayanDSO · 2026-04-29T15:37:48Z

+ * Ignores any matching test defined with `@Flaky` in CI
+ * @see Flaky
+ */
+class IgnoreFlakyTestsInCIRule : TestRule {


idk why are you copying this file

These files aren't in my commits.

BrayanDSO · 2026-04-29T15:38:18Z

+ * @param runnable a function that is expected to throw an exception when executed
+ * @return the exception thrown by [runnable]
+ */
+inline fun <reified T : Throwable> assertThrows(


idk why you are copying this file

These files aren't in my commits. they appear to be pre-existing in the repository or from the base branch

mixelas · 2026-04-29T17:17:07Z

This was initially a placeholder as i didn't think that I had to make the complete HTTPS support. That is why it looks bad and AI generated, as I tried to do a simple fix. I will get on with pushing a PR with complete HTTPS support and give it my best.

mixelas · 2026-05-02T15:59:03Z

@david-allison can you please review again.

david-allison · 2026-05-03T13:49:41Z

The emulator tests are broken (and please remove the merge commit and rebase/force push onto main)

https://github.com/ankidroid/Anki-Android/blob/main/CONTRIBUTING.md#:~:text=AnkiDroid%20expects%20use%20of%20git%20rebase%20and%20git%20push%20%2D%2Dforce%20to%20PR%20branches.%20PRs%20with%20merge%20commits%20will%20not%20be%20merged.

david-allison · 2026-05-03T17:11:01Z

@@ -0,0 +1,36 @@
+/*
+ *  Copyright (c) 2024 David Allison <davidallisongithub@gmail.com>


Please remove these files, they shouldn't have been committed. testutils and testlib have been replaced with test fixtures

BrayanDSO · 2026-05-04T12:27:56Z

 import timber.log.Timber

 abstract class CardViewerViewModel(
+    app: Application,


Following the code, the Application dependency is only necessary to pass it to AnkiServer -> SslUtil so it can get a File from the cache directory.

Instead of all that, get the file on the fragment and pass it down. That way we avoid the Application dependency

… androidTest AnkiAssert

david-allison · 2026-05-04T23:01:37Z

+    init {
+        // Enable HTTPS if context was provided (Issue #15991)
+        // This allows WebView to load cards without cleartext restrictions on modern Android
+        if (isHttps && sslContext != null) {


logically, it feels like isHttps doesn't need to exist. If it's true, then cacheDir is not null, which means sslContext is non-null

Ah, i see, I removed the separate isHttps flag and now derive the scheme from the SSL context itself, so the server is HTTPS only when SSL initialization succeeds

david-allison · 2026-05-04T23:02:25Z

 ) : NanoHTTPD(LOCALHOST, port) {
-    fun baseUrl(): String = "http://$LOCALHOST:$listeningPort/"
+    private val isHttps = cacheDir != null
+    private val sslContext = cacheDir?.let { SslUtil.getSSLContext(it) }


What if this throws?

I think i fixed this one as well. I changed SSL initialization to fail safely: if keystore/context creation throws, the server logs the failure and falls back to HTTP instead of crashing on startup.

david-allison · 2026-05-04T23:02:58Z

        savedInstanceState: Bundle?,
    ) {
-        server = AnkiServer(this).also { it.start() }
+        server = AnkiServer(this, requireContext().cacheDir).also { it.start() }


Why cacheDir?

It differs per-profile, is that intended

The OS can wipe is, is that intended?

I used cacheDir because the keystore is a local, disposable artifact for loopback HTTPS. It is profile-scoped, and can be regenerated if the OS clears it. If you’d prefer a more persistent location, I can switch it, but for this use case cacheDir keeps the storage ephemeral.

david-allison · 2026-05-04T23:03:33Z

Heyy... there's a lot of changes here which aren't relevant to the PR. Take a look at this from a reviewer's perspective:

https://github.com/ankidroid/Anki-Android/pull/20885/changes

The 'git' window can show you what's relevant, and you can revert the changes which aren't necessary

This design seems complex. Moving reviewerViewModelFactory to a separate file seems unnecessary.

It might be worth explaining the design, I'm sure it can be simplified

mixelas · 2026-05-05T09:03:30Z

I addressed the points you raised. I inlined the ViewModel creation into the fragments and kept the cache-dir threading as small as possible. The goal was just to pass the local directory through to the server setup without adding extra architectural layers. Let me know if there is anything else that i need to do.

mixelas added 2 commits April 28, 2026 21:32

fix: Refactor AnkiServer for future HTTPS support (Issue ankidroid#15991

f757daf

)

fix: Simplify unit tests to match placeholder implementation

2e16da8

david-allison reviewed Apr 28, 2026

View reviewed changes

mixelas added 2 commits April 28, 2026 23:56

feat: Added implementation

a8b1515

fix: Remove Robolectric dependency from unit tests

739549b

BrayanDSO reviewed Apr 29, 2026

View reviewed changes

BrayanDSO added the Needs Author Reply Waiting for a reply from the original author label Apr 29, 2026

Fix HTTPS implementation for Issue ankidroid#15991

ffb28ba

mixelas force-pushed the fix/issue-15991-https-support branch from ba113ab to ffb28ba Compare May 3, 2026 14:09

github-actions Bot added the Has Conflicts label May 3, 2026

david-allison reviewed May 3, 2026

View reviewed changes

mixelas force-pushed the fix/issue-15991-https-support branch from 349aa3e to 7c75875 Compare May 4, 2026 10:12

Fixed code for emulator tests

797d3c6

BrayanDSO requested changes May 4, 2026

View reviewed changes

mixelas force-pushed the fix/issue-15991-https-support branch from 7c75875 to 797d3c6 Compare May 4, 2026 18:07

refactor: pass cacheDir into preview server setup

5731752

mixelas force-pushed the fix/issue-15991-https-support branch from 9ddccc0 to 5731752 Compare May 4, 2026 18:24

mixelas added 2 commits May 4, 2026 22:13

style: add GPLv3 headers to new viewmodel factories; remove duplicate…

45d31d2

… androidTest AnkiAssert

remove duplicate IgnoreFlakyTestsInCIRule

154263a

david-allison reviewed May 4, 2026

View reviewed changes

refactor: simplify HTTPS server setup and remove duplicate test utils

263bf19

		@@ -0,0 +1,78 @@
		/*
		* Copyright (c) 2024 AnkiDroid Contributors

		@@ -0,0 +1,36 @@
		/*
		* Copyright (c) 2024 David Allison <davidallisongithub@gmail.com>

Uh oh!

Conversation

mixelas commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

BrayanDSO left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mixelas commented Apr 29, 2026

Uh oh!

mixelas commented May 2, 2026

Uh oh!

david-allison commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-allison commented May 4, 2026

Uh oh!

mixelas commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mixelas commented Apr 28, 2026 •

edited

Loading

david-allison commented May 3, 2026 •

edited

Loading