-
Notifications
You must be signed in to change notification settings - Fork 117
Added new exposeTaffyHeaders #451
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
Added new exposeTaffyHeaders #451
Conversation
…e headers should be written
✅ Deploy Preview for taffy-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
atuttle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall a fantastic PR. One little note for the docs.
|
Will this be something that can be toggled and enabled via a web request. If I were testing something and wanted to enable all the headers (which are now globally disabled), would there be a way to pass a "secret" URL param or header to enable "exposeTaffyHeaders" during a request? I'm inquiring because I currently use browser extensions (like Requestly) to inject HTTP request headers (with encrypted values) to configured hostnames to enable extra features - like enabling debugging, A/B testing, enable feature flags, etc. |
|
Since it's a setting, you can use the environmental settings to enable it by environment, so you could enable it for a "dev" environment, but leave it disabled for production. If you do nothing, it has the same behavior as before. However, given that Taffy stores settings in the application scope, having it behavior differently on a request-by-request nature isn't really possible. |
|
I'd like both options to be available so I don't have to choose between two different environments. If something doesn't seem to be working correctly in production, I'd like the option to pass a secret param to enable the headers to assist in troubleshooting. Similar request override features already exist for application settings... See I reported an issue regarding ColdFusion content flushing occurring before Taffy could add headers Taffy resulting in CF errors. Instead of using the built-in functions, Taffy could collect them (regardless of whether As an example, our web application framework collects all informational (non-essential) header data and outputs them at the end of the request IF the content hasn't already been flushed. This also enables us to negate or overwrite any headers as needed. |
|
I'd argue that you can certainly code this behavior if it's important to you. Add a feature so you can toggle the behavior. You would just need to change the I'm personally not a fan of URL parameters that can alter the behavior of the REST API, because that leaves you open to unexpected issues. For you use case, you might just want to just leave the headers enabled so there's no change in behavior and the headers are always sent. I just didn't want the headers, because they leak information that could be used as an attack vector (i.e. they reveal the REST API is using Taffy). |
|
Oh, I should add, since I refactored the adding of Taffy headers to it's own function, you could just overwrite the method for your implementation. This is the default implementation: You could just override that in your Application.cfc and use something like: Then Taffy would ignore the |
|
Alternatively, instead of adding the information to the headers, you could redirect the output to a log. Lots of options. |
This PR add support for a new
exposeTaffyHeaderssetting, which istrueby default to preserve backwards compatibility.When set to
false, it will prevent the following headers from being sent in the HTTP response:X-TAFFY-RELOADEDX-TIME-TO-RELOADX-TIME-IN-PARSEX-TIME-IN-ONTAFFYREQUESTX-TIME-IN-RESOURCEX-TIME-IN-CACHE-CHECKX-TIME-IN-CACHE-GETX-TIME-IN-CACHE-SAVEX-TIME-IN-SERIALIZEX-TIME-IN-TAFFYX-TIME-IN-ONTAFFYREQUESTENDI also updated the documentation to document what each of these headers means (at least based on my understanding).
My intention was to include unit tests for this, but I was running into tons of issues trying to get the unit tests running in Commandbox. The documentation here seems be very outdated.