From 5ee6dd23cd93385b75c11909627677af8bd4469a Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 24 Jan 2021 12:05:05 -0600 Subject: [PATCH] Fix a bunch of various nits --- .gitignore | 2 +- CHANGELOG.md | 18 ++++- build.gradle | 2 + src/main/kotlin/mdnet/Constants.kt | 2 +- src/main/kotlin/mdnet/MangaDexClient.kt | 6 +- src/main/kotlin/mdnet/ServerManager.kt | 6 +- src/main/kotlin/mdnet/cache/ImageStorage.kt | 37 +++++----- .../mdnet/metrics/GeoIpMetricsFilter.kt | 5 +- .../kotlin/mdnet/netty/ApplicationNetty.kt | 74 +++++++++++++++++-- src/main/resources/logback.xml | 2 + .../kotlin/mdnet/server/ImageServerTest.kt | 5 ++ 11 files changed, 122 insertions(+), 37 deletions(-) diff --git a/.gitignore b/.gitignore index f43d849..9bb14a2 100644 --- a/.gitignore +++ b/.gitignore @@ -107,7 +107,7 @@ log/** images/** *.db -*settings.yaml +settings.yaml /cache docker/data diff --git a/CHANGELOG.md b/CHANGELOG.md index a35ada0..260d9d8 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Fixed -- [2021-01-23] Fix sample docker-compose.yml [@_tde9] ### Security -## [2.0.0-rc1] - 2020-01-23 +## [2.0.0-rc2] - 2021-01-24 +### Added +- [2021-01-24] Epoll and IOUring HTTP transports [@carbotaniuman]. + +### Changed +- [2021-01-23] Log files are now UTF-8 [@_tde9] + +### Fixed +- [2021-01-23] Fix sample docker-compose.yml [@_tde9] +- [2021-01-24] Fix another race condition [@carbotaniuman] +- [2021-01-23] Fix validating small (<1024MiB) caches [@_tde9] + +## [2.0.0-rc1] - 2021-01-23 This release contains many breaking changes! Of note are the changes to the cache folders, database location, and settings format. ### Added - [2021-01-23] Added `external_max_kilobits_per_second` config option [@carbotaniuman]. @@ -280,7 +291,8 @@ This release contains many breaking changes! Of note are the changes to the cach ### Fixed - [2020-06-11] Tweaked logging configuration to reduce log file sizes by [@carbotaniuman]. -[Unreleased]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/2.0.0-rc1...HEAD +[Unreleased]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/2.0.0-rc2...HEAD +[2.0.0-rc2]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/2.0.0-rc1...2.0.0-rc2 [2.0.0-rc1]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/1.2.4...2.0.0-rc1 [1.2.4]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/1.2.3...1.2.4 [1.2.3]: https://gitlab.com/mangadex/mangadex_at_home/-/compare/1.2.2...1.2.3 diff --git a/build.gradle b/build.gradle index c30284f..fdc19e4 100644 --- a/build.gradle +++ b/build.gradle @@ -39,6 +39,8 @@ dependencies { implementation group: "org.http4k", name: "http4k-client-apache", version: "$http_4k_version" implementation group: "org.http4k", name: "http4k-metrics-micrometer", version: "$http_4k_version" implementation group: "org.http4k", name: "http4k-server-netty", version: "$http_4k_version" + implementation group: "io.netty", name: "netty-transport-native-epoll", version: "4.1.58.Final", classifier: "linux-x86_64" + implementation group: "io.netty.incubator", name: "netty-incubator-transport-native-io_uring", version: "0.0.3.Final", classifier: "linux-x86_64" testImplementation group: "org.http4k", name: "http4k-testing-kotest", version: "$http_4k_version" runtimeOnly group: "io.netty", name: "netty-tcnative-boringssl-static", version: "2.0.34.Final" diff --git a/src/main/kotlin/mdnet/Constants.kt b/src/main/kotlin/mdnet/Constants.kt index 5a5078b..3118009 100644 --- a/src/main/kotlin/mdnet/Constants.kt +++ b/src/main/kotlin/mdnet/Constants.kt @@ -21,7 +21,7 @@ package mdnet import java.time.Duration object Constants { - const val CLIENT_BUILD = 21 + const val CLIENT_BUILD = 22 @JvmField val MAX_AGE_CACHE: Duration = Duration.ofDays(14) diff --git a/src/main/kotlin/mdnet/MangaDexClient.kt b/src/main/kotlin/mdnet/MangaDexClient.kt index 8e4bbc4..5a3dbd6 100644 --- a/src/main/kotlin/mdnet/MangaDexClient.kt +++ b/src/main/kotlin/mdnet/MangaDexClient.kt @@ -188,9 +188,9 @@ class MangaDexClient(private val settingsFile: File, databaseFile: File, cacheFo } private fun validateSettings(settings: ClientSettings) { -// if (settings.maxCacheSizeInMebibytes < 1024) { -// throw ClientSettingsException("Config Error: Invalid max cache size, must be >= 1024 MiB (1GiB)") -// } + if (settings.maxCacheSizeInMebibytes < 1024) { + throw ClientSettingsException("Config Error: Invalid max cache size, must be >= 1024 MiB (1GiB)") + } fun isSecretValid(clientSecret: String): Boolean { return Pattern.matches("^[a-zA-Z0-9]{$CLIENT_KEY_LENGTH}$", clientSecret) diff --git a/src/main/kotlin/mdnet/ServerManager.kt b/src/main/kotlin/mdnet/ServerManager.kt index 4f8a5f1..98ab203 100644 --- a/src/main/kotlin/mdnet/ServerManager.kt +++ b/src/main/kotlin/mdnet/ServerManager.kt @@ -201,7 +201,7 @@ class ServerManager( if (newSettings != null) { LOGGER.info { "Server settings received: $newSettings" } - warmBasedOnSettings(newSettings) + warnBasedOnSettings(newSettings) if (!state.settings.logicalEqual(newSettings)) { LOGGER.info { "Doing internal restart of HTTP server to refresh settings" } @@ -221,7 +221,7 @@ class ServerManager( val remoteSettings = backendApi.loginToControl() ?: throw RuntimeException("Failed to get a login response from server") LOGGER.info { "Server settings received: $remoteSettings" } - warmBasedOnSettings(remoteSettings) + warnBasedOnSettings(remoteSettings) val server = getServer( storage, @@ -287,7 +287,7 @@ class ServerManager( LOGGER.info { "Image server has shut down" } } - private fun warmBasedOnSettings(settings: RemoteSettings) { + private fun warnBasedOnSettings(settings: RemoteSettings) { if (settings.latestBuild > Constants.CLIENT_BUILD) { LOGGER.warn { "Outdated build detected! Latest: ${settings.latestBuild}, Current: ${Constants.CLIENT_BUILD}" diff --git a/src/main/kotlin/mdnet/cache/ImageStorage.kt b/src/main/kotlin/mdnet/cache/ImageStorage.kt index b004186..3e42458 100644 --- a/src/main/kotlin/mdnet/cache/ImageStorage.kt +++ b/src/main/kotlin/mdnet/cache/ImageStorage.kt @@ -207,11 +207,7 @@ class ImageStorage( return null } - return try { - WriterImpl(id, metadata) - } catch (e: FileAlreadyExistsException) { - null - } + return WriterImpl(id, metadata) } private fun deleteImage(id: String) { @@ -317,15 +313,28 @@ class ImageStorage( } } catch (e: SQLIntegrityConstraintViolationException) { // someone got to us before this (TOCTOU) + // there are 2 situations here + // one is that the + // other write died in between writing the DB and + // moving the file + // the other is that we have raced and the other + // is about to write the file + // we handle this below + } + + try { + Files.move( + tempPath, + getPath(id), + StandardCopyOption.ATOMIC_MOVE + ) + } catch (e: FileAlreadyExistsException) { + // the file already exists + // so we must lost the race + // delete our local copy abort() return false } - - Files.move( - tempPath, - getPath(id), - StandardCopyOption.ATOMIC_MOVE - ) return true } @@ -333,12 +342,6 @@ class ImageStorage( stream.flush() stream.close() Files.deleteIfExists(tempPath) - - // remove the database entry if it somehow exists - // this really shouldn't happen but just in case - database.delete(DbImage) { - DbImage.id eq id - } } } diff --git a/src/main/kotlin/mdnet/metrics/GeoIpMetricsFilter.kt b/src/main/kotlin/mdnet/metrics/GeoIpMetricsFilter.kt index 05b103c..8780616 100644 --- a/src/main/kotlin/mdnet/metrics/GeoIpMetricsFilter.kt +++ b/src/main/kotlin/mdnet/metrics/GeoIpMetricsFilter.kt @@ -24,6 +24,7 @@ import com.maxmind.geoip2.DatabaseReader import com.maxmind.geoip2.exception.GeoIp2Exception import io.micrometer.prometheus.PrometheusMeterRegistry import mdnet.logging.debug +import mdnet.logging.warn import org.apache.commons.compress.archivers.tar.TarArchiveInputStream import org.apache.commons.io.IOUtils import org.http4k.core.Filter @@ -67,9 +68,9 @@ class GeoIpMetricsFilter( } } catch (e: GeoIp2Exception) { // do not disclose ip here, for privacy of logs - LOGGER.warn("Cannot resolve the country of the request's IP!") + LOGGER.warn { "Cannot resolve the country of the request's IP!" } } catch (e: UnknownHostException) { - LOGGER.warn("Cannot resolve source IP of the request!") + LOGGER.warn { "Cannot resolve source IP of the request!" } } } } diff --git a/src/main/kotlin/mdnet/netty/ApplicationNetty.kt b/src/main/kotlin/mdnet/netty/ApplicationNetty.kt index 792bb33..1e7065a 100644 --- a/src/main/kotlin/mdnet/netty/ApplicationNetty.kt +++ b/src/main/kotlin/mdnet/netty/ApplicationNetty.kt @@ -21,6 +21,9 @@ package mdnet.netty import io.netty.bootstrap.ServerBootstrap import io.netty.channel.* +import io.netty.channel.epoll.Epoll +import io.netty.channel.epoll.EpollEventLoopGroup +import io.netty.channel.epoll.EpollServerSocketChannel import io.netty.channel.nio.NioEventLoopGroup import io.netty.channel.socket.SocketChannel import io.netty.channel.socket.nio.NioServerSocketChannel @@ -34,6 +37,9 @@ import io.netty.handler.timeout.WriteTimeoutException import io.netty.handler.timeout.WriteTimeoutHandler import io.netty.handler.traffic.GlobalTrafficShapingHandler import io.netty.handler.traffic.TrafficCounter +import io.netty.incubator.channel.uring.IOUring +import io.netty.incubator.channel.uring.IOUringEventLoopGroup +import io.netty.incubator.channel.uring.IOUringServerSocketChannel import io.netty.util.concurrent.DefaultEventExecutorGroup import mdnet.Constants import mdnet.data.Statistics @@ -57,17 +63,72 @@ import java.security.cert.X509Certificate import java.util.concurrent.atomic.AtomicReference import javax.net.ssl.SSLException +interface NettyTransport { + val masterGroup: EventLoopGroup + val workerGroup: EventLoopGroup + val factory: ChannelFactory + + fun shutdownGracefully() { + masterGroup.shutdownGracefully() + workerGroup.shutdownGracefully() + } + + private class NioTransport : NettyTransport { + override val masterGroup = NioEventLoopGroup() + override val workerGroup = NioEventLoopGroup() + override val factory = ChannelFactory { NioServerSocketChannel() } + } + + private class EpollTransport : NettyTransport { + override val masterGroup = EpollEventLoopGroup() + override val workerGroup = EpollEventLoopGroup() + override val factory = ChannelFactory { EpollServerSocketChannel() } + } + + private class IOUringTransport : NettyTransport { + override val masterGroup = IOUringEventLoopGroup() + override val workerGroup = IOUringEventLoopGroup() + override val factory = ChannelFactory { IOUringServerSocketChannel() } + } + + companion object { + private val LOGGER = LoggerFactory.getLogger(NettyTransport::class.java) + + fun bestForPlatform(): NettyTransport { + if (IOUring.isAvailable()) { + LOGGER.info("Using IOUring transport") + return IOUringTransport() + } else { + LOGGER.info(IOUring.unavailabilityCause()) { + "IOUring transport not available" + } + } + + if (Epoll.isAvailable()) { + LOGGER.info("Using Epoll transport") + return EpollTransport() + } else { + LOGGER.info(Epoll.unavailabilityCause()) { + "Epoll transport not available" + } + } + + LOGGER.info("Using Nio transport") + return NioTransport() + } + } +} + class Netty(private val tls: TlsCert, private val serverSettings: ServerSettings, private val statistics: AtomicReference) : ServerConfig { override fun toServer(httpHandler: HttpHandler): Http4kServer = object : Http4kServer { - private val masterGroup = NioEventLoopGroup() - private val workerGroup = NioEventLoopGroup() + private val transport = NettyTransport.bestForPlatform() private val executor = DefaultEventExecutorGroup(serverSettings.threads) private lateinit var closeFuture: ChannelFuture private lateinit var address: InetSocketAddress private val burstLimiter = object : GlobalTrafficShapingHandler( - workerGroup, serverSettings.maxKilobitsPerSecond * 1000L / 8L, 0, 50 + transport.workerGroup, serverSettings.maxKilobitsPerSecond * 1000L / 8L, 0, 50 ) { override fun doAccounting(counter: TrafficCounter) { statistics.getAndUpdate { @@ -87,8 +148,8 @@ class Netty(private val tls: TlsCert, private val serverSettings: ServerSettings .build() val bootstrap = ServerBootstrap() - bootstrap.group(masterGroup, workerGroup) - .channelFactory(ChannelFactory { NioServerSocketChannel() }) + bootstrap.group(transport.masterGroup, transport.workerGroup) + .channelFactory(transport.factory) .childHandler(object : ChannelInitializer() { public override fun initChannel(ch: SocketChannel) { ch.pipeline().addLast("ssl", sslContext.newHandler(ch.alloc())) @@ -133,8 +194,7 @@ class Netty(private val tls: TlsCert, private val serverSettings: ServerSettings override fun stop() = apply { closeFuture.cancel(false) - workerGroup.shutdownGracefully() - masterGroup.shutdownGracefully() + transport.shutdownGracefully() executor.shutdownGracefully() } diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 21fd852..dbb7552 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -13,6 +13,7 @@ --> + UTF-8 %d{yyyy-MM-dd HH:mm:ss} %-5level %logger{36} - %msg%n @@ -23,6 +24,7 @@ + UTF-8 %d{yyyy-MM-dd HH:mm:ss} %-5level %logger{36} - %msg%n diff --git a/src/test/kotlin/mdnet/server/ImageServerTest.kt b/src/test/kotlin/mdnet/server/ImageServerTest.kt index 7eeacfd..f3e5241 100644 --- a/src/test/kotlin/mdnet/server/ImageServerTest.kt +++ b/src/test/kotlin/mdnet/server/ImageServerTest.kt @@ -289,6 +289,11 @@ class TokenVerifierTest : FreeSpec() { val response = handler(Request(Method.GET, "/a/data/02181a8f5fe8cd408720a771dd129fd8/T2.png")) response.shouldNotHaveStatus(Status.OK) } + + "invalid token should fail" { + val response = handler(Request(Method.GET, "/MTIzM2Vhd2Z3YWVmbG1pbzJuM29pNG4yaXAzNG1wMSwyWzMscHdxZWVlZWVlZXBscWFkcVt3ZGwxWzJsM3BbMWwycFsxZSxwMVssZmRbcGF3LGZwW2F3ZnBbLA==/data/02181a8f5fe8cd408720a771dd129fd8/T2.png")) + response.shouldNotHaveStatus(Status.OK) + } } } }