twitter / finatra

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

FileResolver should only search in localDocRoot when localDocRoot is specified #566

Closed thinkiny closed 3 years ago

thinkiny commented 3 years ago

Is your feature request related to a problem? Please describe. current strategy is try absolute path first, then under local DocumentRoot

Describe the solution you'd like when user specify localDocRoot, only search in that folder is intuitive, also it can avoid security problem. use localDocumentRoot to replace / in absolute path.

burtomaps commented 3 years ago

في أحد، ٨ أغسطس، ٢٠٢١ في ٨:٢١ م، كتب thinkiny @.***>:

Is your feature request related to a problem? Please describe. current strategy is try absolute path first, then under local DocumentRoot

Describe the solution you'd like when user specify localDocRoot, only search in that folder is intuitive, also it can avoid security problem. use localDocumentRoot to replace / in absolute path.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/twitter/finatra/issues/566, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARPLYKPEPF4XHPNMZCAKNIDT324JVANCNFSM5BYVXJBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

mosesn commented 3 years ago

@thinkiny It looks like this is long-standing behavior: https://github.com/twitter/finatra/commit/2116200a8d87ad4e337a4289a0928d03116a2c0a

Can you elaborate on your concern here? I'm a bit worried that because this is used primarily for local development that it will be unsafe to change the behavior here, so I'm inclined not to change the behavior. However, if it's causing a serious security issue for you, we can look into it.

thinkiny commented 3 years ago

seems like a feature, thanks for explaining