twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 408 forks source link

ResponseBuilder.fileOrIndex doesn't check when file exists or not #360

Closed AlexITC closed 7 years ago

AlexITC commented 8 years ago

ResponseBuilder.fileOrIndex(file, index) doesn't validate when file exists.

Expected behavior

When file doesn't exists, Finatra should provide index.

Actual behavior

The method fileOrIndex provides file when the file name contains a file extension and doesn't validate its existence.

In this case the file name can have a valid extension and don't be present in the file system, causing Finatra to return http status 404 (Not found).

Steps to reproduce the behavior

  get("/:*") { request: Request =>
    response.ok.fileOrIndex(
      filePath = request.params("*"),
      indexPath = "index.html")
  }
AlexITC commented 8 years ago

The expected behavior I wrote is what I understand the method should do.

If this is the case, I would be happy to send a pull request.

Can someone confirm?

Thanks.

ryanoneill commented 8 years ago

Hi @AlexITC, thanks for the submission. In my opinion, it should return the index page as well, and not a 404. I'm currently checking our internal codebase to ensure that changing the semantics here will not break existing code. I'm also going to get the opinion of a few other folks here before proceeding. Thanks.

AlexITC commented 8 years ago

I've finally updated my pull request with some tests.