Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rework symlink test #445

Merged
merged 4 commits into from
Jan 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion http-tests/src/test/resources/dirWithLink/linked-dir

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,21 @@ package directives

import java.io.File

import org.apache.pekko
import pekko.http.scaladsl.settings.RoutingSettings
import pekko.http.scaladsl.testkit.RouteTestTimeout

import scala.concurrent.duration._
import scala.util.Properties
import org.scalatest.{ Inside, Inspectors }
import pekko.http.scaladsl.model.MediaTypes._

import org.apache.pekko
import pekko.http.impl.util._
import pekko.http.scaladsl.model._
import pekko.http.scaladsl.model.MediaTypes._
import pekko.http.scaladsl.model.headers._
import pekko.http.impl.util._
import pekko.http.scaladsl.model.Uri.Path
import pekko.http.scaladsl.settings.RoutingSettings
import pekko.http.scaladsl.testkit.RouteTestTimeout
import pekko.testkit._

import org.scalatest.{ Inside, Inspectors }

class FileAndResourceDirectivesSpec extends RoutingSpec with Inspectors with Inside {

// operations touch files, can be randomly hit by slowness
Expand Down Expand Up @@ -210,16 +211,6 @@ class FileAndResourceDirectivesSpec extends RoutingSpec with Inspectors with Ins
headers should contain(`Last-Modified`(DateTime(lastModified)))
}
}
"not follow symbolic links to find a file" in {
EventFilter.warning(pattern = ".* points to a location that is not part of .*", occurrences = 1).intercept {
Get("linked-dir/empty.pdf") ~> _getFromDirectory("dirWithLink") ~> check {
handled shouldBe false
/* TODO: resurrect following links under an option
responseAs[String] shouldEqual "123"
mediaType shouldEqual `application/pdf`*/
}
}
}
}

"getFromResource" should {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* license agreements; and to You under the Apache License, version 2.0:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* This file is part of the Apache Pekko project, which was derived from Akka.
*/

/*
* Copyright (C) 2009-2022 Lightbend Inc. <https://www.lightbend.com>
*/

package org.apache.pekko.http.scaladsl.server
package directives

import java.io.File
import java.nio.file.{ Files, Paths }

import scala.concurrent.duration._

import org.apache.pekko
import pekko.http.scaladsl.testkit.RouteTestTimeout
import pekko.testkit._

import org.scalatest.{ BeforeAndAfterAll, Inside, Inspectors }

class FileAndResourceDirectivesSymlinkSpec extends RoutingSpec
with Inspectors with Inside with BeforeAndAfterAll {

// need to serve from the src directory, when sbt copies the resource directory over to the
// target directory it will resolve symlinks in the process
val testRoot = new File("http-tests/src/test/resources")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better with Files.path, and yes Winodws is using different separator

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So TIL that Windows has symlinks but only for Windows 10/11. I don't know how this factors into everything, my initial inclination is to disable the test for Windows since this seems to only really be testing behaviour specific to Posix systems.

@He-Pin Can you confirm if the test passes on your machine (assuming all of the Windows related path separator issues have been solved)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is copied from an existing test. The test either works or fails on Windows - either way noone is complaining about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that Windows Java installs know how to apply the file path even if it doesn't use Windows idioms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@He-Pin could you try running sbt http-tests/test? It doesn't take that long compared to similar commands in main pekko repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@He-Pin I created #446 - is ok to say that you are not blocking this? I would like to get this merged so that I can do an RC2 for the Pekko HTTP 1.0.1 release.

require(testRoot.exists(), s"testRoot was not found at ${testRoot.getAbsolutePath}")

val tempDir = Files.createTempDirectory("pekko-http-symlink-test")
tempDir.toFile.deleteOnExit()
val dirWithLink = new File(tempDir.toFile, "dirWithLink")
dirWithLink.mkdir()
val symlink = Files.createSymbolicLink(
Paths.get(dirWithLink.getAbsolutePath, "linked-dir"),
new File(testRoot, "subDirectory").toPath.toAbsolutePath)

override def afterAll(): Unit = {
super.afterAll()
Files.deleteIfExists(symlink)
Files.deleteIfExists(dirWithLink.toPath)
Files.deleteIfExists(tempDir)
}

// operations touch files, can be randomly hit by slowness
implicit val routeTestTimeout: RouteTestTimeout = RouteTestTimeout(3.seconds.dilated)

override def testConfigSource = super.testConfigSource ++ """
pekko.http.routing.range-coalescing-threshold = 1
"""

"getFromDirectory" should {
def _getFromDirectory() = getFromDirectory(dirWithLink.getCanonicalPath)

"not follow symbolic links to find a file" in {
Files.isSymbolicLink(symlink) shouldBe true
EventFilter.warning(pattern = ".* points to a location that is not part of .*", occurrences = 1).intercept {
Get("linked-dir/empty.pdf") ~> _getFromDirectory() ~> check {
handled shouldBe false
/* TODO: resurrect following links under an option
responseAs[String] shouldEqual "123"
mediaType shouldEqual `application/pdf`*/
}
}
}
}
}
Loading