Skip to content

Commit

Permalink
Make sure javascript_include_tag/stylesheet_link_tag does not append …
Browse files Browse the repository at this point in the history 
…".js" or ".css" onto external urls [#1664 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information
@matthewrudy @lifo
matthewrudy authored and lifo committed Aug 5, 2009
1 parent bfafe8c commit 64268a0
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 12 deletions.
2 changes: 2 additions & 0 deletions actionpack/CHANGELOG
View file
@@ -1,5 +1,7 @@
*Edge*

* Make sure javascript_include_tag/stylesheet_link_tag does not append ".js" or ".css" onto external urls. #1664 [Matthew Rudy Jacobs]

* Ruby 1.9: fix Content-Length for multibyte send_data streaming. #2661 [Sava Chankov]

* Ruby 1.9: ERB template encoding using a magic comment at the top of the file. [Jeremy Kemper]
Expand Down
24 changes: 14 additions & 10 deletions actionpack/lib/action_view/helpers/asset_tag_helper.rb
View file
Expand Up @@ -171,15 +171,15 @@ def auto_discovery_link_tag(type = :rss, url_options = {}, tag_options = {})
end

# Computes the path to a javascript asset in the public javascripts directory.
# If the +source+ filename has no extension, .js will be appended.
# If the +source+ filename has no extension, .js will be appended (except for explicit URIs)
# Full paths from the document root will be passed through.
# Used internally by javascript_include_tag to build the script path.
#
# ==== Examples
# javascript_path "xmlhr" # => /javascripts/xmlhr.js
# javascript_path "dir/xmlhr.js" # => /javascripts/dir/xmlhr.js
# javascript_path "/dir/xmlhr" # => /dir/xmlhr.js
# javascript_path "http://www.railsapplication.com/js/xmlhr" # => http://www.railsapplication.com/js/xmlhr.js
# javascript_path "http://www.railsapplication.com/js/xmlhr" # => http://www.railsapplication.com/js/xmlhr
# javascript_path "http://www.railsapplication.com/js/xmlhr.js" # => http://www.railsapplication.com/js/xmlhr.js
def javascript_path(source)
compute_public_path(source, 'javascripts', 'js')
Expand Down Expand Up @@ -337,16 +337,16 @@ def self.reset_javascript_include_default #:nodoc:
end

# Computes the path to a stylesheet asset in the public stylesheets directory.
# If the +source+ filename has no extension, <tt>.css</tt> will be appended.
# If the +source+ filename has no extension, <tt>.css</tt> will be appended (except for explicit URIs).
# Full paths from the document root will be passed through.
# Used internally by +stylesheet_link_tag+ to build the stylesheet path.
#
# ==== Examples
# stylesheet_path "style" # => /stylesheets/style.css
# stylesheet_path "dir/style.css" # => /stylesheets/dir/style.css
# stylesheet_path "/dir/style.css" # => /dir/style.css
# stylesheet_path "http://www.railsapplication.com/css/style" # => http://www.railsapplication.com/css/style.css
# stylesheet_path "http://www.railsapplication.com/css/style.js" # => http://www.railsapplication.com/css/style.css
# stylesheet_path "http://www.railsapplication.com/css/style" # => http://www.railsapplication.com/css/style
# stylesheet_path "http://www.railsapplication.com/css/style.css" # => http://www.railsapplication.com/css/style.css
def stylesheet_path(source)
compute_public_path(source, 'stylesheets', 'css')
end
Expand Down Expand Up @@ -629,11 +629,11 @@ def compute_public_path(source, dir, ext = nil, include_host = true)
has_request = @controller.respond_to?(:request)

source_ext = File.extname(source)[1..-1]
if ext && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}"))))
if ext && !is_uri?(source) && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}"))))
source += ".#{ext}"
end

unless source =~ %r{^[-a-z]+://}
unless is_uri?(source)
source = "/#{dir}/#{source}" unless source[0] == ?/

source = rewrite_asset_path(source)
Expand All @@ -645,10 +645,10 @@ def compute_public_path(source, dir, ext = nil, include_host = true)
end
end

if include_host && source !~ %r{^[-a-z]+://}
if include_host && !is_uri?(source)
host = compute_asset_host(source)

if has_request && !host.blank? && host !~ %r{^[-a-z]+://}
if has_request && !host.blank? && !is_uri?(host)
host = "#{@controller.request.protocol}#{host}"
end

Expand All @@ -658,6 +658,10 @@ def compute_public_path(source, dir, ext = nil, include_host = true)
end
end

def is_uri?(path)
path =~ %r{^[-a-z]+://}
end

# Pick an asset host for this source. Returns +nil+ if no host is set,
# the host if no wildcard is set, the host interpolated with the
# numbers 0-3 if it contains <tt>%d</tt> (the number is the source hash mod 4),
Expand Down Expand Up @@ -798,7 +802,7 @@ def asset_file_path(path)
end

def asset_file_path!(path)
unless path =~ %r{^[-a-z]+://}
unless is_uri?(path)
absolute_path = asset_file_path(path)
raise(Errno::ENOENT, "Asset file not found at '#{absolute_path}'" ) unless File.exist?(absolute_path)
return absolute_path
Expand Down
8 changes: 6 additions & 2 deletions actionpack/test/template/asset_tag_helper_test.rb
View file
Expand Up @@ -83,7 +83,10 @@ def teardown
%(javascript_include_tag(:all)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>),
%(javascript_include_tag(:all, :recursive => true)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>),
%(javascript_include_tag(:defaults, "bank")) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
%(javascript_include_tag("bank", :defaults)) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>)
%(javascript_include_tag("bank", :defaults)) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),

%(javascript_include_tag("http://example.com/all")) => %(<script src="http://example.com/all" type="text/javascript"></script>),
%(javascript_include_tag("http://example.com/all.js")) => %(<script src="http://example.com/all.js" type="text/javascript"></script>),
}

StylePathToTag = {
Expand Down Expand Up @@ -111,7 +114,8 @@ def teardown
%(stylesheet_link_tag(:all, :media => "all")) => %(<link href="/stylesheets/bank.css" media="all" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/robber.css" media="all" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/version.1.0.css" media="all" rel="stylesheet" type="text/css" />),
%(stylesheet_link_tag("random.styles", "/elsewhere/file")) => %(<link href="/stylesheets/random.styles" media="screen" rel="stylesheet" type="text/css" />\n<link href="/elsewhere/file.css" media="screen" rel="stylesheet" type="text/css" />),

%(stylesheet_link_tag("http://www.example.com/styles/style")) => %(<link href="http://www.example.com/styles/style.css" media="screen" rel="stylesheet" type="text/css" />)
%(stylesheet_link_tag("http://www.example.com/styles/style")) => %(<link href="http://www.example.com/styles/style" media="screen" rel="stylesheet" type="text/css" />),
%(stylesheet_link_tag("http://www.example.com/styles/style.css")) => %(<link href="http://www.example.com/styles/style.css" media="screen" rel="stylesheet" type="text/css" />),
}

ImagePathToTag = {
Expand Down

0 comments on commit 64268a0

Please sign in to comment.