Skip to content

Commit 653f030

Browse files
rtibblesclaude
andcommitted
Address second round of PR review feedback
- Fix race condition in KolibriServerService.startHttpServer(): set isRunning before thread.start() to prevent duplicate server threads - Call resetServerState() in KolibriServerService.onDestroy() so ViewModel reflects actual server state - Harden WebView security: disable third-party cookies, file/content access, and mixed content mode (unnecessary for local HTTP server) - Remove dead Django migration loader patch (Django 3.2 does not need it) - Rename PatchKolibriTask to StreamlineKolibriTask and deduplicate extraction logic into shared static method - Rename tar/patched to tar/extracted throughout - Remove obsolete py2only exclude from tar extraction Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 432e438 commit 653f030

6 files changed

Lines changed: 49 additions & 94 deletions

File tree

.gitignore

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ app/src/main/res/values-*/strings.xml
77
tmpenv
88
tmphome/
99

10-
# Kolibri tar files and patches
10+
# Kolibri tar files and extracted source
1111
tar/
12-
tar/patched/
12+
tar/extracted/
1313

1414
# File format for signing keys
1515
*.jks

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ clean:
5050

5151
.PHONY: clean-tar
5252
clean-tar:
53-
rm -rf tar/patched
53+
rm -rf tar/extracted
5454
mkdir -p tar
5555

5656
.PHONY: get-tar
@@ -242,7 +242,7 @@ help:
242242
@echo ""
243243
@echo "Kolibri Source:"
244244
@echo " get-tar - Download Kolibri tar (use: make get-tar tar=URL)"
245-
@echo " clean-tar - Remove patched Kolibri directory"
245+
@echo " clean-tar - Remove extracted Kolibri directory"
246246
@echo ""
247247
@echo "Environment Variables:"
248248
@echo " ANDROID_SDK_ROOT - Android SDK location (default: ./android_root)"

app/build.gradle

Lines changed: 33 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -23,67 +23,51 @@ spotless {
2323
import org.gradle.api.tasks.*
2424
import javax.inject.Inject
2525

26-
// Task to extract and patch Kolibri tar file (replicates Makefile install-tar)
27-
abstract class PatchKolibriTask extends DefaultTask {
26+
// Task to extract Kolibri tar file, stripping unnecessary packages
27+
abstract class StreamlineKolibriTask extends DefaultTask {
2828
@InputFile
2929
abstract RegularFileProperty getTarFile()
3030

3131
@OutputDirectory
32-
abstract DirectoryProperty getPatchedDir()
32+
abstract DirectoryProperty getExtractedDir()
3333

3434
@Inject
3535
abstract ExecOperations getExecOperations()
3636

37-
@TaskAction
38-
void patchKolibri() {
39-
def tarFile = getTarFile().get().asFile
40-
def patchedDir = getPatchedDir().get().asFile
37+
/** Extract Kolibri tar, cleaning the output directory and stripping unnecessary packages. */
38+
static void extract(File tarFile, File extractedDir, def execOps) {
39+
println "Extracting Kolibri from ${tarFile.name}"
4140

42-
if (!tarFile.exists()) {
43-
throw new GradleException("Kolibri tar file not found: ${tarFile}")
41+
if (extractedDir.exists()) {
42+
extractedDir.deleteDir()
4443
}
44+
extractedDir.mkdirs()
4545

46-
println "Patching Kolibri from ${tarFile.name}"
47-
48-
// Clean and create patched directory
49-
if (patchedDir.exists()) {
50-
patchedDir.deleteDir()
51-
}
52-
patchedDir.mkdirs()
53-
54-
// Extract tar file, excluding problematic packages
55-
execOperations.exec {
46+
execOps.exec {
5647
commandLine 'tar', 'xzf', tarFile.absolutePath,
57-
'--exclude=kolibri/dist/py2only*',
5848
'--exclude=kolibri/dist/cext/*',
5949
'--exclude=kolibri/dist/ifaddr*',
60-
'--directory=' + patchedDir.absolutePath,
50+
'--directory=' + extractedDir.absolutePath,
6151
'--strip-components=1'
6252
}
6353

64-
// Patch Django to allow .pyc migration files
65-
// This is critical because Chaquopy compiles .py files and deletes them
66-
def djangoLoader = new File(patchedDir, 'kolibri/dist/django/db/migrations/loader.py')
67-
if (djangoLoader.exists()) {
68-
def content = djangoLoader.text
69-
content = content.replace(
70-
'if name.endswith(".py"):',
71-
'if name.endswith(".py") or name.endswith(".pyc"):'
72-
)
73-
djangoLoader.text = content
74-
println "Patched Django migrations loader"
75-
} else {
76-
throw new GradleException("Could not find Django loader.py to patch")
77-
}
54+
println "Kolibri extracted successfully at ${extractedDir.absolutePath}"
55+
}
7856

79-
println "Kolibri patched successfully at ${patchedDir.absolutePath}"
57+
@TaskAction
58+
void streamlineKolibri() {
59+
def tarFile = getTarFile().get().asFile
60+
if (!tarFile.exists()) {
61+
throw new GradleException("Kolibri tar file not found: ${tarFile}")
62+
}
63+
extract(tarFile, getExtractedDir().get().asFile, execOperations)
8064
}
8165
}
8266

83-
tasks.register('patchKolibriTar', PatchKolibriTask) {
67+
tasks.register('streamlineKolibriTar', StreamlineKolibriTask) {
8468
def tarFile = file('../tar').listFiles()?.find { it.name.endsWith('.tar.gz') && it.name.startsWith('kolibri') }
8569
getTarFile().set(tarFile)
86-
getPatchedDir().set(file('../tar/patched'))
70+
getExtractedDir().set(file('../tar/extracted'))
8771
}
8872

8973
// Determine which Python to use for build scripts
@@ -93,52 +77,22 @@ if (venvPython.exists()) {
9377
buildPythonExecutable = venvPython.absolutePath
9478
}
9579

96-
// Ensure Kolibri tar is patched during configuration phase
80+
// Ensure Kolibri tar is extracted during configuration phase
9781
// This is necessary so version can be calculated before tasks run
98-
def patchedKolibriDir = file('../tar/patched')
82+
def extractedKolibriDir = file('../tar/extracted')
9983
def tarFile = file('../tar').listFiles()?.find { it.name.endsWith('.tar.gz') && it.name.startsWith('kolibri') }
10084

101-
if (tarFile != null && tarFile.exists() && !new File(patchedKolibriDir, 'kolibri/VERSION').exists()) {
102-
println "Extracting and patching Kolibri tar during configuration..."
103-
104-
// Clean and create patched directory
105-
if (patchedKolibriDir.exists()) {
106-
patchedKolibriDir.deleteDir()
107-
}
108-
patchedKolibriDir.mkdirs()
109-
110-
// Extract tar file, excluding problematic packages
111-
exec {
112-
commandLine 'tar', 'xzf', tarFile.absolutePath,
113-
'--exclude=kolibri/dist/py2only*',
114-
'--exclude=kolibri/dist/cext/*',
115-
'--exclude=kolibri/dist/ifaddr*',
116-
'--directory=' + patchedKolibriDir.absolutePath,
117-
'--strip-components=1'
118-
}
119-
120-
// Patch Django to allow .pyc migration files
121-
def djangoLoader = new File(patchedKolibriDir, 'kolibri/dist/django/db/migrations/loader.py')
122-
if (djangoLoader.exists()) {
123-
def content = djangoLoader.text
124-
content = content.replace(
125-
'if name.endswith(".py"):',
126-
'if name.endswith(".py") or name.endswith(".pyc"):'
127-
)
128-
djangoLoader.text = content
129-
println "Patched Django migrations loader"
130-
}
131-
132-
println "Kolibri patched successfully"
85+
if (tarFile != null && tarFile.exists() && !new File(extractedKolibriDir, 'kolibri/VERSION').exists()) {
86+
StreamlineKolibriTask.extract(tarFile, extractedKolibriDir, this)
13387
}
13488

13589
// Calculate version name and code
13690
def calculatedVersionName
13791
def calculatedVersionCode
13892

139-
if (patchedKolibriDir.exists() && patchedKolibriDir.isDirectory()) {
93+
if (extractedKolibriDir.exists() && extractedKolibriDir.isDirectory()) {
14094
// Read VERSION_NAME from Kolibri's VERSION file
141-
def kolibriVersionFile = new File(patchedKolibriDir, 'kolibri/VERSION')
95+
def kolibriVersionFile = new File(extractedKolibriDir, 'kolibri/VERSION')
14296
if (!kolibriVersionFile.exists()) {
14397
throw new GradleException("Kolibri VERSION file not found at: ${kolibriVersionFile}")
14498
}
@@ -211,21 +165,21 @@ abstract class GenerateKolibriStringsTask extends DefaultTask {
211165
}
212166

213167
tasks.register('generateKolibriStrings', GenerateKolibriStringsTask) {
214-
dependsOn patchKolibriTar
168+
dependsOn streamlineKolibriTar
215169
getScriptFile().set(file('../scripts/create_strings.py'))
216170
getResDir().set(file('src/main/res'))
217171
// Version text input is sufficient - when it changes, task reruns
218172
getVersionText().set(calculatedVersionName)
219173
getPythonExecutable().set(buildPythonExecutable)
220174
}
221175

222-
// Make sure Kolibri is patched before Python requirements are generated
176+
// Make sure Kolibri is extracted before Python requirements are generated
223177
afterEvaluate {
224178
tasks.matching { task ->
225179
task.name.contains('PythonRequirements') ||
226180
task.name.contains('extractPythonBuildPackages')
227181
}.all { task ->
228-
task.dependsOn patchKolibriTar
182+
task.dependsOn streamlineKolibriTar
229183
}
230184

231185
// Generate strings before resources are processed or mapped
@@ -278,8 +232,8 @@ android {
278232

279233
pip {
280234
// Install Kolibri from patched tar file
281-
// The patchKolibriTar task extracts and patches the tar before this runs
282-
install "../tar/patched"
235+
// The streamlineKolibriTar task extracts the tar before this runs
236+
install "../tar/extracted"
283237
install "-r", "../requirements.txt"
284238
}
285239
}

app/src/main/java/org/learningequality/Kolibri/KolibriServerService.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ private synchronized void startHttpServer() {
3838
return;
3939
}
4040

41+
isRunning = true;
4142
serverThread =
4243
new Thread(
4344
() -> {
4445
try {
4546
Log.i(TAG, "Starting Kolibri HTTP server");
46-
isRunning = true;
4747

4848
Python py = Python.getInstance();
4949
PyObject mainModule = py.getModule("main");
@@ -70,6 +70,7 @@ public void onDestroy() {
7070
Log.d(TAG, "KolibriServerService onDestroy");
7171

7272
isRunning = false;
73+
KolibriServerViewModel.getInstance().resetServerState();
7374

7475
// Call Python to stop the server gracefully
7576
try {

app/src/main/java/org/learningequality/Kolibri/WebViewActivity.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,18 @@ private void setupWebView() {
8989
// Enable cookies and ensure persistence
9090
CookieManager cookieManager = CookieManager.getInstance();
9191
cookieManager.setAcceptCookie(true);
92-
cookieManager.setAcceptThirdPartyCookies(webView, true);
92+
cookieManager.setAcceptThirdPartyCookies(webView, false);
9393

9494
// Enable DOM storage for Service Worker
9595
settings.setDomStorageEnabled(true);
9696
settings.setJavaScriptEnabled(true);
9797

98-
// Enable Service Worker support (mixed content for local HTTP server)
99-
settings.setMixedContentMode(WebSettings.MIXED_CONTENT_ALWAYS_ALLOW);
98+
// Local HTTP server only — no mixed content needed
99+
settings.setMixedContentMode(WebSettings.MIXED_CONTENT_NEVER_ALLOW);
100100

101-
// Enable modern web features
102-
settings.setAllowFileAccess(true);
103-
settings.setAllowContentAccess(true);
101+
// No file:// or content:// access needed — Kolibri loads via http://127.0.0.1
102+
settings.setAllowFileAccess(false);
103+
settings.setAllowContentAccess(false);
104104

105105
// Set WebChromeClient for fullscreen video
106106
webView.setWebChromeClient(new KolibriWebChromeClient(this, fullscreenContainer));

scripts/create_strings.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
import xml.etree.ElementTree as ET
99
from importlib import resources
1010

11-
# Add patched Kolibri tar to Python path
12-
PATCHED_TAR_PATH = os.path.join(os.path.dirname(__file__), "../tar/patched")
13-
if os.path.exists(PATCHED_TAR_PATH):
14-
sys.path.insert(0, PATCHED_TAR_PATH)
11+
# Add extracted Kolibri tar to Python path
12+
EXTRACTED_TAR_PATH = os.path.join(os.path.dirname(__file__), "../tar/extracted")
13+
if os.path.exists(EXTRACTED_TAR_PATH):
14+
sys.path.insert(0, EXTRACTED_TAR_PATH)
1515

1616

1717
# By default we will map the locale code to e.g. "en-us" to "en-rUS"

0 commit comments

Comments
 (0)