Skip to content

Loading…

Allow scheme-relative URLs #1433

Closed
wants to merge 3 commits into from

3 participants

@serveace

URLs starting with "//" will choose the scheme of the current page. This allows more flexibility when loading content from CDNs.

See discussion at https://groups.google.com/forum/?fromgroups=#!topic/liftweb/4N0-Rkhb_1c .

serveace added some commits
@serveace serveace Allow scheme-relative urls
URLs starting with "//" will choose the scheme of the current page.  This allows more flexibility when loading content from CDNs.
2b3e0b7
@serveace serveace Don't rewrite scheme-relative URLs 1ad4223
@serveace serveace commented on the diff
web/webkit/src/main/scala/net/liftweb/http/Req.scala
@@ -544,6 +544,7 @@ object Req {
private def _fixHref(contextPath: String, v: Seq[Node], fixURL: Boolean, rewrite: Box[String => String]): Text = {
val hv = v.text
val updated = if (hv.startsWith("/") &&
+ !hv.startsWith("//") &&

From what I can tell, it looks like it's necessary to run this check in both places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Shadowfiend
Lift Web Framework member

Nice. Does this sit ready as far as you've been able to test, or are more changes needed to get everything working with scheme-relative URLs?

@serveace

I don't currently have any environment to build lift, so I'll have to do that before I run a couple tests.

@Shadowfiend
Lift Web Framework member

Ok, cool. You seem to have an application that would use this at least a little bit, so I think it'll make a good real-world test to ensure this works.

That said, you may want to merge this with lift_26_dev when testing, which is the 2.6-SNAPSHOT branch until we release Lift 2.5. That will let you publish-local from sbt and be able to depend in your app on 2.6-SNAPSHOT to use the resulting packages.

@serveace

I was able to test this tonight.

I tested with and without a context, and with a mixture of different types of URLs.

Looks like everything worked as expected.

@Shadowfiend
Lift Web Framework member

Ok, rock on. I'll try and build it and run it for some of my apps tonight to see if things look good there, and if so I'll merge 'er up!

@fmpwizard
Lift Web Framework member

rebased to master

@fmpwizard fmpwizard closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 16, 2013
  1. @serveace

    Allow scheme-relative urls

    serveace committed
    URLs starting with "//" will choose the scheme of the current page.  This allows more flexibility when loading content from CDNs.
  2. @serveace
Commits on Apr 17, 2013
  1. @serveace
Showing with 8 additions and 0 deletions.
  1. +6 −0 contributors.md
  2. +2 −0 web/webkit/src/main/scala/net/liftweb/http/Req.scala
View
6 contributors.md
@@ -137,3 +137,9 @@ Taylor Leese
### Email: ###
tleese22 at gmail dot com
+
+### Name: ###
+Austen Holmes
+
+### Email: ###
+austen dot holmes at gmail dot com
View
2 web/webkit/src/main/scala/net/liftweb/http/Req.scala
@@ -544,6 +544,7 @@ object Req {
private def _fixHref(contextPath: String, v: Seq[Node], fixURL: Boolean, rewrite: Box[String => String]): Text = {
val hv = v.text
val updated = if (hv.startsWith("/") &&
+ !hv.startsWith("//") &&

From what I can tell, it looks like it's necessary to run this check in both places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
!LiftRules.excludePathFromContextPathRewriting.vend(hv)) contextPath + hv else hv
Text(if (fixURL && rewrite.isDefined &&
@@ -551,6 +552,7 @@ object Req {
!updated.startsWith("javascript:") &&
!updated.startsWith("http://") &&
!updated.startsWith("https://") &&
+ !updated.startsWith("//") &&
!updated.startsWith("#"))
rewrite.openOrThrowException("legacy code").apply(updated) else updated)
}
Something went wrong with that request. Please try again.