Skip to content
This repository

Remove userinfo from base_tag #437

Closed
wants to merge 1 commit into from

5 participants

Brian Duggan Sebastian Riedel Tianon Gravi Joel Berger Marcus Ramberg
Brian Duggan

Hi again (re : github issue 436),

Here's a pull request to remove userinfo from the base_tag helper. This seems like a good security fix; putting userinfo inside a web page means credentials may not only be in caches but also some browsers (firefox 17) actually display the credentials as part of the link destination (on the bottom of the screen). Am happy to bring this up on the mailing list if you think more discussion is warranted.

thanks again
Brian

Sebastian Riedel
Owner

What are the disadvantages of this change? And what is the helper used for these days, good URLs are generated with url_for and friends, wouldn't it make more sense to just deprecate it?

Sebastian Riedel kraih closed this in 5e59bd9 January 04, 2013
Brian Duggan

I can't think of any disadvantages; putting credentials into a base tag seems like unexpected behavior.

But I think you're right, if every URL comes from url_for, link_to, etc, base_tag shouldn't be necessary. (I just noticed that url_for also explicitly removes userinfo https://github.com/kraih/mojo/blob/master/lib/Mojolicious/Controller.pm#L405
).

I can't think of a good reason not to deprecate it, though it may be convenient for those cases where url_for can't be used (e.g. generating vast numbers of links or dynamically generating them).

Tianon Gravi

How are URLs within Javascript files normally handled (especially with regards to AJAX/AJAJ calls)? We've been using base_tag for making this work properly, but would love to hear a better method.

Joel Berger
Collaborator

I typically write my pure javascript in separate files in terms of a function argument, then when called from the template I can use the url_for helper in the arguments. Of course this is just my take, but it makes things very simple for me.

Marcus Ramberg
Collaborator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Jan 04, 2013
remove userinfo from base_tag helper caf5177
This page is out of date. Refresh to see the latest.
2  lib/Mojolicious/Plugin/TagHelpers.pm
@@ -15,7 +15,7 @@ sub register {
15 15
 
16 16
   # Add "base_tag" helper
17 17
   $app->helper(
18  
-    base_tag => sub { _tag('base', href => shift->req->url->base, @_) });
  18
+    base_tag => sub { _tag('base', href => shift->req->url->base->clone->userinfo(undef), @_) });
19 19
 
20 20
   # Add "checkbox" helper
21 21
   $app->helper(check_box =>
9  t/mojolicious/lite_app.t
@@ -516,6 +516,9 @@ get '/dynamic/inline' => sub {
516 516
   $self->render(inline => 'dynamic inline ' . $dynamic_inline++);
517 517
 };
518 518
 
  519
+# GET /base_tag
  520
+get '/base_tag';
  521
+
519 522
 # Oh Fry, I love you more than the moon, and the stars,
520 523
 # and the POETIC IMAGE NUMBER 137 NOT FOUND
521 524
 my $t = Test::Mojo->new;
@@ -1237,6 +1240,9 @@ $t->get_ok('/dynamic/inline')->status_is(200)
1237 1240
 $t->get_ok('/dynamic/inline')->status_is(200)
1238 1241
   ->content_is("dynamic inline 2\n");
1239 1242
 
  1243
+$t->get_ok( $t->ua->app_url->userinfo('secret:pass')->path('/base_tag') )
  1244
+  ->status_is(200)->content_unlike(qr/secret/);
  1245
+
1240 1246
 done_testing();
1241 1247
 
1242 1248
 __DATA__
@@ -1330,6 +1336,9 @@ Not a favicon!
1330 1336
 %== url_with('/test')->query([foo => undef])
1331 1337
 %== url_with('bartest', test => 23)->query([foo => 'yada'])
1332 1338
 
  1339
+@@ base_tag.html.ep
  1340
+base tag : <%= base_tag %>
  1341
+
1333 1342
 __END__
1334 1343
 This is not a template!
1335 1344
 lalala
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.