FAQ
Reviewers: golang-dev1,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go


Description:
net/url: prepend slash to path in String()

Previously if a path was set manually without a leading /, String()
would not insert the slash when writing it's output. This would lead
to situations where a URL that should be http://www.google.com/search
is output as http://www.google.comsearch

Fixes issue 5927.

Please review this at https://codereview.appspot.com/11698045/

Affected files:
    M src/pkg/net/url/url.go


Index: src/pkg/net/url/url.go
===================================================================
--- a/src/pkg/net/url/url.go
+++ b/src/pkg/net/url/url.go
@@ -459,6 +459,11 @@
       buf.WriteString(h)
      }
     }
+ if u.Path != "" {
+ if u.Path[0] != '/' {
+ buf.WriteByte('/')
+ }
+ }
     buf.WriteString(escape(u.Path, encodePath))
    }
    if u.RawQuery != "" {


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Brad Fitzpatrick at Jul 23, 2013 at 8:39 pm
    "its output".

    Also: if this is an acceptable change, it would need a test.


    On Tue, Jul 23, 2013 at 1:24 PM, wrote:

    Reviewers: golang-dev1,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://code.google.com/p/go


    Description:
    net/url: prepend slash to path in String()

    Previously if a path was set manually without a leading /, String()
    would not insert the slash when writing it's output. This would lead
    to situations where a URL that should be http://www.google.com/search
    is output as http://www.google.comsearch

    Fixes issue 5927.

    Please review this at https://codereview.appspot.**com/11698045/<https://codereview.appspot.com/11698045/>

    Affected files:
    M src/pkg/net/url/url.go


    Index: src/pkg/net/url/url.go
    ==============================**==============================**=======
    --- a/src/pkg/net/url/url.go
    +++ b/src/pkg/net/url/url.go
    @@ -459,6 +459,11 @@
    buf.WriteString(h)
    }
    }
    + if u.Path != "" {
    + if u.Path[0] != '/' {
    + buf.WriteByte('/')
    + }
    + }
    buf.WriteString(escape(u.Path, encodePath))
    }
    if u.RawQuery != "" {


    --

    ---You received this message because you are subscribed to the Google
    Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegroups.com>
    .
    For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out>
    .

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Russ Cox at Jul 23, 2013 at 9:15 pm
    I can't decide if this is correct.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Scottwferg at Jul 23, 2013 at 10:46 pm

    On 2013/07/23 20:39:20, bradfitz wrote:
    "its output".
    Also: if this is an acceptable change, it would need a test.
    Updated for grammar ;)

    Updated with a test, though I'm not sure how I feel about it. Simply
    adding the case to urltests will fail since reflect.DeepEqual() won't
    match. The solution I found is to copy urltests and append this new case
    to a slice called stringtests. If there's design concern over this I can
    move the discussion over to golang-nuts.

    https://codereview.appspot.com/11698045/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Scottwferg at Jul 23, 2013 at 11:01 pm
    Nevermind that, realized there was a bug in that test. I've added the
    specific test case to TestURLString().
    On 2013/07/23 22:46:33, scottferg wrote:
    On 2013/07/23 20:39:20, bradfitz wrote:
    "its output".

    Also: if this is an acceptable change, it would need a test.
    Updated for grammar ;)
    Updated with a test, though I'm not sure how I feel about it. Simply
    adding the
    case to urltests will fail since reflect.DeepEqual() won't match. The
    solution I
    found is to copy urltests and append this new case to a slice called
    stringtests. If there's design concern over this I can move the
    discussion over
    to golang-nuts.


    https://codereview.appspot.com/11698045/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Raff367 at Oct 17, 2013 at 3:55 am
    I belive this fix is not 100% correct (or at least it breaks a use case
    that works in Go < 1.2).

    the following test used to return "a/b/c" and now it returns "/a/b/c" (i.e.
    a relative URL becomes absolute):

    func main() {
         path := "a/b/c"
         u, _ := url.Parse(path)

         fmt.Println("path:", path)
         fmt.Println("url:", u)
    }

    I understand the root problem but I think the correct fix should be to also
    check that the buffer is not empty (i.e. if I do create a URL with
    schema/host and a relative path I DO WANT the "/", but if the URL only has
    the Path set, it should be left alone).

    Also, I don't see any test for this particular use case
    (if you want I can submit a patch for the fix and test case)

    Thanks!

    -- Raffaele


    On Tuesday, July 23, 2013 1:24:21 PM UTC-7, Scott Ferguson wrote:

    Reviewers: golang-dev1,

    Message:
    Hello golan...@googlegroups.com <javascript:>,

    I'd like you to review this change to
    https://code.google.com/p/go


    Description:
    net/url: prepend slash to path in String()

    Previously if a path was set manually without a leading /, String()
    would not insert the slash when writing it's output. This would lead
    to situations where a URL that should be http://www.google.com/search
    is output as http://www.google.comsearch

    Fixes issue 5927.

    Please review this at https://codereview.appspot.com/11698045/

    Affected files:
    M src/pkg/net/url/url.go


    Index: src/pkg/net/url/url.go
    ===================================================================
    --- a/src/pkg/net/url/url.go
    +++ b/src/pkg/net/url/url.go
    @@ -459,6 +459,11 @@
    buf.WriteString(h)
    }
    }
    + if u.Path != "" {
    + if u.Path[0] != '/' {
    + buf.WriteByte('/')
    + }
    + }
    buf.WriteString(escape(u.Path, encodePath))
    }
    if u.RawQuery != "" {

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dave Cheney at Oct 17, 2013 at 3:58 am
    Please open an issue for this regression.
    On 17 Oct 2013, at 12:20, raff367@gmail.com wrote:

    I belive this fix is not 100% correct (or at least it breaks a use case that works in Go < 1.2).

    the following test used to return "a/b/c" and now it returns "/a/b/c" (i.e. a relative URL becomes absolute):

    func main() {
    path := "a/b/c"
    u, _ := url.Parse(path)

    fmt.Println("path:", path)
    fmt.Println("url:", u)
    }

    I understand the root problem but I think the correct fix should be to also check that the buffer is not empty (i.e. if I do create a URL with schema/host and a relative path I DO WANT the "/", but if the URL only has the Path set, it should be left alone).

    Also, I don't see any test for this particular use case
    (if you want I can submit a patch for the fix and test case)

    Thanks!

    -- Raffaele


    On Tuesday, July 23, 2013 1:24:21 PM UTC-7, Scott Ferguson wrote:
    Reviewers: golang-dev1,

    Message:
    Hello golan...@googlegroups.com,

    I'd like you to review this change to
    https://code.google.com/p/go


    Description:
    net/url: prepend slash to path in String()

    Previously if a path was set manually without a leading /, String()
    would not insert the slash when writing it's output. This would lead
    to situations where a URL that should be http://www.google.com/search
    is output as http://www.google.comsearch

    Fixes issue 5927.

    Please review this at https://codereview.appspot.com/11698045/

    Affected files:
    M src/pkg/net/url/url.go


    Index: src/pkg/net/url/url.go
    ===================================================================
    --- a/src/pkg/net/url/url.go
    +++ b/src/pkg/net/url/url.go
    @@ -459,6 +459,11 @@
    buf.WriteString(h)
    }
    }
    + if u.Path != "" {
    + if u.Path[0] != '/' {
    + buf.WriteByte('/')
    + }
    + }
    buf.WriteString(escape(u.Path, encodePath))
    }
    if u.RawQuery != "" {
    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJul 23, '13 at 8:24p
activeOct 17, '13 at 3:58a
posts7
users5
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase