dart-lang / shelf

Web server middleware for Dart
https://pub.dev/packages/shelf
BSD 3-Clause "New" or "Revised" License
921 stars 124 forks source link

Add context to Response for including file and file_not_found to be identified by other shelf Middleware. #436

Open gmpassos opened 3 months ago

gmpassos commented 3 months ago

With the Response.context populated with the processed file, other middleware can manipulate the response in an optimized way and ensure the correct processed file. Without this, another middleware would need to guess the processed file and, in some cases, re-resolve File.stat and fix the directory path to the "index.html" path.

This is the continuation of the PR: https://github.com/dart-lang/shelf/pull/395

(This was needed to release my shelf/master fork for another PR).


kevmoo commented 3 months ago

Need some tests here, sir.

gmpassos commented 3 months ago

Need some tests here, sir.

kevmoo commented 3 months ago

@gmpassos – look at the windows failures. Looks like some path mundging needs to be done

gmpassos commented 3 months ago

It's the path separator

test\symbolic_link_test.dart: access outside of root disabled access real file (failed)
  Expected: {
              'shelf_static:file': 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\dart_test_ee1d415b\\originals/index.html'
            }
    Actual: {
              'shelf_static:file': 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\dart_test_ee1d415b\\originals\\index.html'
            }
     Which: at location ['shelf_static:file'] is 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\dart_test_ee1d415b\\originals\\index.html' instead of 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\dart_test_ee1d415b\\originals/index.html'
kevmoo commented 3 months ago

Yep. You'll need to normalize those to handle windows. Always fun!

gmpassos commented 3 months ago

Should be fixed now.

I was using path.join in a completely wrong way:

p.join(d.sandbox, 'foo/file.txt')

😅

gmpassos commented 3 months ago

...needed an extra fix for createFileHandler, since it derivates the File path from the url.path

kevmoo commented 3 months ago

@devoncarew @natebosch – thoughts?

gmpassos commented 3 months ago

Is it ready for publishing?

kevmoo commented 3 months ago

I'd like other eyes on this change first @gmpassos

gmpassos commented 2 months ago

I'd like other eyes on this change first @gmpassos

Does the code need any adjustments?

Best regards.