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

Efficiency improvements to serve_module_asset() #205

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

monkeysdocode
Copy link
Contributor

Some efficiency improvements when serving module assets - helpful when dealing with larger files.

  1. readfile() vs file_get_contents() streams larger files and avoids the need to read the whole file into memory first.

  2. added 304 Not Modified response to HTTP_IF_MODIFIED_SINCE request. Avoids having to send files if client just wants to check that their cached copy is still valid.

  3. is_file() checks if it's a valid file vs file_exists() which returns true for files and directories

  4. 403 response code to .php and api.json files is the more common response to attempts to access files that shouldn't be

  5. Add Last-Modified header when sending file

Some efficiency improvements when serving module assets - helpful when dealing with larger files.

1) readfile() vs file_get_contents() streams larger files and avoids the need to read the whole file into memory first. 

2) added 304 Not Modified response to HTTP_IF_MODIFIED_SINCE request. Avoids having to send files if client just wants to check that their cached copy is still valid.

3) is_file() checks if it's a valid file vs file_exists() which returns true for files and directories

4) 403 response code to .php and api.json files is the more common response to attempts to access files that shouldn't be

5) Add Last-Modified header when sending file
Add UTF8 charset to $dsn
@trongate
Copy link
Owner

Thanks! I don't entirely understand it. Let me run this past some people who are far smarter than me. I'm sure what you've done is amazing. I'll be in touch shortly. Hail to you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants