Skip to content

Commit

Permalink
fix(kobo): better handling of missing port from Kobo Sync requests
Browse files Browse the repository at this point in the history
  • Loading branch information
gotson committed Sep 4, 2024
1 parent be37127 commit 4cd838a
Show file tree
Hide file tree
Showing 10 changed files with 216 additions and 0 deletions.
2 changes: 2 additions & 0 deletions komga-webui/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -933,8 +933,10 @@
"btn_confirm": "Yes, but only if bigger",
"title": "Regenerate thumbnails"
},
"hint_kobo_port": "Set only in case of sync issues with covers and downloads",
"label_delete_empty_collections": "Delete empty collections after scan",
"label_delete_empty_readlists": "Delete empty read lists after scan",
"label_kobo_port": "Kobo Sync external port",
"label_kobo_proxy": "Proxy Kobo Sync requests to Kobo Store",
"label_rememberme_duration": "Remember me duration (in days)",
"label_server_context_path": "Base URL",
Expand Down
2 changes: 2 additions & 0 deletions komga-webui/src/types/komga-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface SettingsDto {
serverPort: SettingMultiSource<number>,
serverContextPath: SettingMultiSource<string>,
koboProxy: boolean,
koboPort?: number,
}

export interface SettingMultiSource<T> {
Expand All @@ -25,6 +26,7 @@ export interface SettingsUpdateDto {
serverPort?: number,
serverContextPath?: string,
koboProxy?: boolean,
koboPort?: number,
}

export enum ThumbnailSizeDto {
Expand Down
28 changes: 28 additions & 0 deletions komga-webui/src/views/ServerSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,21 @@
:label="$t('server_settings.label_kobo_proxy')"
hide-details
/>
<v-text-field
v-model="form.koboPort"
@input="$v.form.koboPort.$touch()"
@blur="$v.form.koboPort.$touch()"
:error-messages="koboPortErrors"
clearable
:label="$t('server_settings.label_kobo_port')"
:hint="$t('server_settings.hint_kobo_port')"
persistent-hint
type="number"
min="1"
max="65535"
class="mt-4"
/>
</v-col>
</v-row>
<v-row>
Expand Down Expand Up @@ -166,6 +181,7 @@ export default Vue.extend({
serverPort: 25600,
serverContextPath: '',
koboProxy: false,
koboPort: undefined,
},
existingSettings: {} as SettingsDto,
dialogRegenerateThumbnails: false,
Expand All @@ -192,6 +208,10 @@ export default Vue.extend({
contextPath,
},
koboProxy: {},
koboPort: {
minValue: minValue(1),
maxValue: maxValue(65535),
},
},
},
mounted() {
Expand Down Expand Up @@ -230,6 +250,12 @@ export default Vue.extend({
!this.$v?.form?.serverContextPath?.contextPath && errors.push(this.$t('validation.context_path').toString())
return errors
},
koboPortErrors(): string[] {
const errors = [] as string[]
if (!this.$v.form?.koboPort?.$dirty) return errors;
(!this.$v?.form?.koboPort?.minValue || !this.$v?.form?.koboPort?.maxValue) && errors.push(this.$t('validation.tcp_port').toString())
return errors
},
saveDisabled(): boolean {
return this.$v.form.$invalid || !this.$v.form.$anyDirty
},
Expand Down Expand Up @@ -271,6 +297,8 @@ export default Vue.extend({
if (this.$v.form?.koboProxy?.$dirty)
this.$_.merge(newSettings, {koboProxy: this.form.koboProxy})
if (this.$v.form?.koboPort?.$dirty)
this.$_.merge(newSettings, {koboPort: this.form.koboPort})
await this.$komgaSettings.updateSettings(newSettings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ class KomgaSettingsProvider(
serverSettingsDao.saveSetting(Settings.KOBO_PROXY.name, value)
field = value
}

var koboPort: Int? =
serverSettingsDao.getSettingByKey(Settings.KOBO_PORT.name, Int::class.java)
set(value) {
if (value != null)
serverSettingsDao.saveSetting(Settings.KOBO_PORT.name, value)
else
serverSettingsDao.deleteSetting(Settings.KOBO_PORT.name)
field = value
}
}

private enum class Settings {
Expand All @@ -104,4 +114,5 @@ private enum class Settings {
SERVER_PORT,
SERVER_CONTEXT_PATH,
KOBO_PROXY,
KOBO_PORT,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.gotson.komga.infrastructure.web

import jakarta.servlet.FilterChain
import jakarta.servlet.http.HttpServletRequest
import jakarta.servlet.http.HttpServletRequestWrapper
import jakarta.servlet.http.HttpServletResponse
import org.springframework.web.filter.OncePerRequestFilter

class KoboMissingPortFilter(private val koboPortSupplier: () -> Int?) : OncePerRequestFilter() {
companion object {
private val FORWARDED_HEADER_NAMES =
setOf(
"Forwarded",
"X-Forwarded-Host",
"X-Forwarded-Port",
"X-Forwarded-Proto",
"X-Forwarded-Prefix",
"X-Forwarded-Ssl",
"X-Forwarded-For",
)
}

override fun shouldNotFilter(request: HttpServletRequest): Boolean {
// ignore this filter if forwarded headers are present
FORWARDED_HEADER_NAMES.forEach { if (request.getHeader(it) != null) return true }
return false
}

override fun shouldNotFilterAsyncDispatch(): Boolean = false

override fun shouldNotFilterErrorDispatch(): Boolean = false

override fun doFilterInternal(
request: HttpServletRequest,
response: HttpServletResponse,
filterChain: FilterChain,
) {
val wrappedRequest =
try {
KoboMissingPortRequest(request, koboPortSupplier)
} catch (ex: Throwable) {
if (logger.isDebugEnabled) logger.debug("Failed to apply missing port to " + formatRequest(request), ex)
response.sendError(HttpServletResponse.SC_BAD_REQUEST)
return
}
filterChain.doFilter(wrappedRequest, response)
}

override fun doFilterNestedErrorDispatch(
request: HttpServletRequest,
response: HttpServletResponse,
filterChain: FilterChain,
) {
doFilterInternal(request, response, filterChain)
}

private fun formatRequest(request: HttpServletRequest) = "HTTP ${request.method} \"${request.requestURI}\""

private class KoboMissingPortRequest(request: HttpServletRequest, val port: () -> Int?) : HttpServletRequestWrapper(request) {
override fun getServerPort() = port() ?: request.serverPort
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package org.gotson.komga.infrastructure.web

import jakarta.annotation.PostConstruct
import org.gotson.komga.infrastructure.configuration.KomgaSettingsProvider
import org.springframework.boot.web.servlet.FilterRegistrationBean
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.Ordered
import org.springframework.web.filter.ForwardedHeaderFilter

@Configuration
class KoboMissingPortFilterConfiguration(
private val komgaSettingsProvider: KomgaSettingsProvider,
private val serverSettings: WebServerEffectiveSettings,
private val forwardedHeaderFilter: FilterRegistrationBean<ForwardedHeaderFilter>,
) {
@Bean
fun koboMissingPortFilter(): FilterRegistrationBean<out KoboMissingPortFilter> =
FilterRegistrationBean(
KoboMissingPortFilter { komgaSettingsProvider.koboPort ?: serverSettings.effectiveServerPort },
).apply {
addUrlPatterns(
"/kobo/*",
)
setName("koboMissingPortFilter")
order = Ordered.HIGHEST_PRECEDENCE
}

@PostConstruct
fun adjustForwardHeaderFilterOrder() {
// the ForwardHeaderFilter must be after the KoboMissingPortFilter, as the latter's detection is based on forwarded headers
// that the former will remove
forwardedHeaderFilter.order = Ordered.HIGHEST_PRECEDENCE + 1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class SettingsController(
SettingMultiSource(configServerPort, komgaSettingsProvider.serverPort, serverSettings.effectiveServerPort),
SettingMultiSource(configServerContextPath, komgaSettingsProvider.serverContextPath, serverSettings.effectiveServletContextPath),
komgaSettingsProvider.koboProxy,
komgaSettingsProvider.koboPort,
)

@PatchMapping
Expand All @@ -60,5 +61,6 @@ class SettingsController(
if (newSettings.isSet("serverContextPath")) komgaSettingsProvider.serverContextPath = newSettings.serverContextPath

newSettings.koboProxy?.let { komgaSettingsProvider.koboProxy = it }
if (newSettings.isSet("koboPort")) komgaSettingsProvider.koboPort = newSettings.koboPort
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ data class SettingsDto(
val serverPort: SettingMultiSource<Int?>,
val serverContextPath: SettingMultiSource<String?>,
val koboProxy: Boolean,
val koboPort: Int?,
)

data class SettingMultiSource<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ class SettingsUpdateDto {
}

var koboProxy: Boolean? = null

@get:Positive
@get:Max(65535)
var koboPort: Int?
by Delegates.observable(null) { prop, _, _ ->
isSet[prop.name] = true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@ import org.gotson.komga.domain.service.SeriesLifecycle
import org.gotson.komga.infrastructure.configuration.KomgaSettingsProvider
import org.gotson.komga.infrastructure.kobo.KoboHeaders
import org.gotson.komga.infrastructure.kobo.KomgaSyncTokenGenerator
import org.gotson.komga.infrastructure.web.WebServerEffectiveSettings
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.MethodSource
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.beans.factory.annotation.Value
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.http.HttpHeaders
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.get
import java.util.stream.Stream

@SpringBootTest(properties = ["komga.kobo.sync-item-limit=1"])
@AutoConfigureMockMvc(printOnlyOnFailure = false)
Expand All @@ -39,6 +46,7 @@ class KoboControllerTest(
@Autowired private val mediaRepository: MediaRepository,
@Autowired private val komgaSyncTokenGenerator: KomgaSyncTokenGenerator,
@Autowired private val komgaSettingsProvider: KomgaSettingsProvider,
@Autowired private val serverSettings: WebServerEffectiveSettings,
) {
private val library1 = makeLibrary()
private val user1 =
Expand Down Expand Up @@ -128,4 +136,62 @@ class KoboControllerTest(
komgaSettingsProvider.koboProxy = false
}
}

@Nested
inner class HostHeader(
@Value("\${server.port:#{null}}") private val configServerPort: Int?,
) {
@ParameterizedTest
@MethodSource("headers")
fun `given partial host header when getting initialization then img urls are correct`(
hostHeader: String,
expected: String,
extraHeaders: Map<String, String>?,
koboPort: Int?,
) {
// ServletWebServerInitializedEvent is not triggered during tests
serverSettings.effectiveServerPort = configServerPort

val oldPort = komgaSettingsProvider.koboPort
koboPort?.let { komgaSettingsProvider.koboPort = it }

try {
mockMvc.get("/kobo/$apiKey/v1/initialization") {
header(HttpHeaders.HOST, hostHeader)
extraHeaders?.forEach { (h, v) -> header(h, v) }
}
.andExpect {
jsonPath("Resources.image_host") { value(expected) }
}.andReturn()
} finally {
komgaSettingsProvider.koboPort = oldPort
}
}

private fun headers(): Stream<Arguments> =
Stream.of(
Arguments.of("127.0.0.1", "http://127.0.0.1:$configServerPort", null, null),
Arguments.of("localhost", "http://localhost:$configServerPort", null, null),
Arguments.of(
"127.0.0.1",
"https://demo.komga.org",
mapOf(
"X-Forwarded-Proto" to "https",
"X-Forwarded-Host" to "demo.komga.org",
),
null,
),
Arguments.of("127.0.0.1", "http://127.0.0.1:8085", null, 8085),
Arguments.of("localhost", "http://localhost:8085", null, 8085),
Arguments.of(
"127.0.0.1",
"https://demo.komga.org",
mapOf(
"X-Forwarded-Proto" to "https",
"X-Forwarded-Host" to "demo.komga.org",
),
8085,
),
)
}
}

0 comments on commit 4cd838a

Please sign in to comment.