shelf icon indicating copy to clipboard operation
shelf copied to clipboard

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

Open gmpassos opened this issue 1 year ago • 11 comments

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).


  • [✅ ] I’ve reviewed the contributor guide and applied the relevant portions to this PR.

gmpassos avatar Jun 14 '24 05:06 gmpassos

Need some tests here, sir.

kevmoo avatar Jun 14 '24 05:06 kevmoo

Need some tests here, sir.

  • Implemented context for directories.
  • Tests: done ✅

gmpassos avatar Jun 14 '24 08:06 gmpassos

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

kevmoo avatar Jun 14 '24 17:06 kevmoo

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'
  

gmpassos avatar Jun 14 '24 18:06 gmpassos

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

kevmoo avatar Jun 14 '24 18:06 kevmoo

Should be fixed now.

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

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

😅

gmpassos avatar Jun 14 '24 18:06 gmpassos

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

gmpassos avatar Jun 14 '24 18:06 gmpassos

@devoncarew @natebosch – thoughts?

kevmoo avatar Jun 14 '24 19:06 kevmoo

Is it ready for publishing?

gmpassos avatar Jun 24 '24 00:06 gmpassos

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

kevmoo avatar Jun 24 '24 05:06 kevmoo

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

Does the code need any adjustments?

Best regards.

gmpassos avatar Jul 20 '24 23:07 gmpassos

Any update?

Best regards.

gmpassos avatar Nov 19 '24 19:11 gmpassos