Merge branch '2500-logging' into 'master'

Don't log URLs, filenames or site content in release builds

Closes #2500

See merge request fdroid/fdroidclient!1171
This commit is contained in:
Hans-Christoph Steiner
2022-12-30 15:18:13 +00:00
5 changed files with 21 additions and 26 deletions

View File

@@ -132,7 +132,7 @@ public class HttpDownloader constructor(
// calculatedEtag == expectedETag (ETag calculated from server response matches expected ETag)
if (!headInfo.eTagChanged || calculatedEtag == expectedETag) {
// ETag has not changed, don't download again
log.info { "${request.indexFile.name} cached, not downloading." }
log.debug { "${request.indexFile.name} cached, not downloading." }
hasChanged = false
return
}
@@ -145,24 +145,20 @@ public class HttpDownloader constructor(
var resumable = false
val fileLength = outputFile.length()
if (fileLength > (fileSize ?: -1)) {
if (!outputFile.delete()) log.warn {
"Warning: " + outputFile.absolutePath + " not deleted"
}
if (!outputFile.delete()) log.warn { "Warning: outputFile not deleted" }
} else if (fileLength == fileSize && outputFile.isFile) {
log.info { "Already have outputFile, not download. ${outputFile.absolutePath}" }
log.debug { "Already have outputFile, not downloading: ${outputFile.name}" }
return // already have it!
} else if (fileLength > 0) {
resumable = true
}
log.info { "downloading ${request.indexFile.name} (is resumable: $resumable)" }
log.debug { "Downloading ${request.indexFile.name} (is resumable: $resumable)" }
runBlocking {
try {
downloadFromBytesReceiver(resumable)
} catch (e: NoResumeException) {
require(resumable) { "Got $e even though download was not resumable" }
if (!outputFile.delete()) log.warn {
"Warning: " + outputFile.absolutePath + " not deleted"
}
if (!outputFile.delete()) log.warn { "Warning: outputFile not deleted" }
downloadFromBytesReceiver(false)
}
}

View File

@@ -62,24 +62,20 @@ public class HttpDownloaderV2 constructor(
var resumable = false
val fileLength = outputFile.length()
if (fileLength > (request.indexFile.size ?: -1)) {
if (!outputFile.delete()) log.warn {
"Warning: " + outputFile.absolutePath + " not deleted"
}
if (!outputFile.delete()) log.warn { "Warning: outputFile not deleted" }
} else if (fileLength == request.indexFile.size && outputFile.isFile) {
log.info { "Already have outputFile, not download. ${outputFile.absolutePath}" }
log.debug { "Already have outputFile, not downloading: ${outputFile.name}" }
return // already have it!
} else if (fileLength > 0) {
resumable = true
}
log.info { "downloading ${request.indexFile.name} (is resumable: $resumable)" }
log.debug { "Downloading ${request.indexFile.name} (is resumable: $resumable)" }
runBlocking {
try {
downloadFromBytesReceiver(resumable)
} catch (e: NoResumeException) {
require(resumable) { "Got $e even though download was not resumable" }
if (!outputFile.delete()) log.warn {
"Warning: " + outputFile.absolutePath + " not deleted"
}
if (!outputFile.delete()) log.warn { "Warning: outputFile not deleted" }
downloadFromBytesReceiver(false)
}
}

View File

@@ -31,7 +31,8 @@ public class HttpGlideUrlLoader(
height: Int,
options: Options,
): LoadData<InputStream> {
log.warn { "Not using mirrors when loading $glideUrl" }
if (log.isDebugEnabled) log.warn { "Not using mirrors when loading $glideUrl" }
else log.warn { "Not using mirrors for load" }
return LoadData(glideUrl, HttpFetcher(httpManager, glideUrl, proxyGetter()))
}

View File

@@ -96,7 +96,7 @@ public open class HttpManager @JvmOverloads constructor(
val response: HttpResponse = try {
mirrorChooser.mirrorRequest(request) { mirror, url ->
resetProxyIfNeeded(request.proxy, mirror)
log.info { "HEAD $url" }
log.debug { "HEAD $url" }
httpClient.head(url) {
addQueryParameters()
// add authorization header from username / password if set
@@ -106,7 +106,7 @@ public open class HttpManager @JvmOverloads constructor(
}
}
} catch (e: ResponseException) {
log.warn(e) { "Error getting HEAD" }
log.warn { "Error getting HEAD: ${e.response.status}" }
if (e.response.status == NotFound) throw NotFoundException()
return null
}
@@ -153,7 +153,7 @@ public open class HttpManager @JvmOverloads constructor(
skipFirstBytes: Long,
): HttpStatement {
resetProxyIfNeeded(request.proxy, mirror)
log.info { "GET $url" }
log.debug { "GET $url" }
return httpClient.prepareGet(url) {
addQueryParameters()
// add authorization header from username / password if set
@@ -209,13 +209,13 @@ public open class HttpManager @JvmOverloads constructor(
private fun resetProxyIfNeeded(proxyConfig: ProxyConfig?, mirror: Mirror? = null) {
// force no-proxy when trying to hit a local mirror
val newProxy = if (mirror.isLocal() && proxyConfig != null) {
if (currentProxy != null) log.info {
if (currentProxy != null) log.debug {
"Forcing mirror to null, because mirror is local: $mirror"
}
null
} else proxyConfig
if (currentProxy != newProxy) {
log.info { "Switching proxy from [$currentProxy] to [$newProxy]" }
log.debug { "Switching proxy from [$currentProxy] to [$newProxy]" }
httpClient.close()
httpClient = getNewHttpClient(newProxy)
}

View File

@@ -68,9 +68,11 @@ internal abstract class MirrorChooserImpl : MirrorChooser {
}
private fun throwOnLastMirror(e: Exception, wasLastMirror: Boolean) {
log.warn(e) {
if (wasLastMirror) "Last mirror, rethrowing..."
else "Trying other mirror now..."
log.info {
val info = if (e is ResponseException) e.response.status.toString()
else e::class.simpleName ?: ""
if (wasLastMirror) "Last mirror, rethrowing... ($info)"
else "Trying other mirror now... ($info)"
}
if (wasLastMirror) throw e
}