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

XDEBUG_FILE in cookie #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

XDEBUG_FILE in cookie #164

wants to merge 1 commit into from

Conversation

MoleDJ
Copy link

@MoleDJ MoleDJ commented Mar 16, 2015

Add new format to xdebug.trace_output_name to allow set the filename from $_COOKIE "XDEBUG_FILE"

%K filename (from $_COOKIE if set) trace.%K trace.filename_in_set_in_the_XDEBUG_FILE_cookie.xt

@derickr
Copy link
Contributor

derickr commented Mar 19, 2015

I like this, but what's the reason why it is called XDEBUG_FILE ? It's just an arbitrary value that you can get from the GET parameters into the trace/profile filenames. Would it not make sense to pick a name that doesn't include FILE?

It would also be great if you could file an issue at http://bugs.xdebug.org as feature request, as per http://xdebug.org/contributing.php You probably should create a feature branch as well, instead of using "master".

@derickr
Copy link
Contributor

derickr commented Mar 24, 2015

Hey @MoleDJ — are you still interested in this?

@MoleDJ
Copy link
Author

MoleDJ commented Mar 26, 2015

Sorry for the delay, I already interested in this. the reason I want to add this is that I need to change the trace filename from the request side and I cannot use the url. In my exact setup, I need diferent trace file per each request I do, and I want to have this files named as I want so I can then match the results from webgrind to a database.

Sorry If I didn't put the change in the correct place, is my first contribution in github :)

@derickr
Copy link
Contributor

derickr commented Mar 27, 2015

Okay - I get that, but does the cookie needs to be called XDEBUG_FILE - instead of for example XDEBUG_EXTRA_INFO ? I can imagine other want to send something else and not a filename.

@MoleDJ
Copy link
Author

MoleDJ commented Mar 29, 2015

Oh, cookie name not need to be XDEBUG_FILE. Can be extra_info or any other best fit on your name schema.

@derickr derickr force-pushed the master branch 2 times, most recently from 8e87621 to 6acaefc Compare June 15, 2015 20:18
@andypost
Copy link
Contributor

andypost commented Feb 6, 2017

And here should option to disable this behavior

Copy link
Contributor

@derickr derickr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've some questions/requests.

sess_name = "XDEBUG_FILE";

if (PG(http_globals)[TRACK_VARS_COOKIE] &&
zend_hash_find(Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_COOKIE]), sess_name, strlen(sess_name) + 1, (void **) &data) == SUCCESS &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer sizeof(sess_name) here, and probably I'd prefer that line 656 and 658 are the same.

I would also argue that the name should not be XDEBUG_FILE, but instead XDEBUG_EXTRA_INFO.

What would be even better if it was an #define somewhere as a "constant".


if (PG(http_globals)[TRACK_VARS_COOKIE] &&
zend_hash_find(Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_COOKIE]), sess_name, strlen(sess_name) + 1, (void **) &data) == SUCCESS &&
Z_STRLEN_PP(data) < 100 /* Prevent any unrealistically long data being set as filename */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value "100" should also be a constant.

@@ -650,6 +650,28 @@ int xdebug_format_output_filename(char **filename, char *format, char *script_na
}
} break;

case 'K': { /* XDEBUG_FILE in cookie */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you pick "K" ? Can we make it "X", according to the xdebug_eXtra_info ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm new to this

@derickr
Copy link
Contributor

derickr commented Mar 6, 2017

Sorry, I'm just looking again at old(er) PRs. @andypost , why should there be an option to disable this?

@andypost
Copy link
Contributor

@derickr I misunderstood, as I see it escapes .. and /

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