Bug when navigating between builds

[daniel.galan]

Hello,

when I click on "previous build" oder "next build" the buildnumber is added to the url, responding in a 404. Here an example:

I'm watching build 696 and want to go back to 695 the buildnumber is added before the old buildnumber to the url:
http://ourbuildserver/hudson/job/ourproject/695/696/
goind forward is resulting in this url:
http://ourbuildserver/hudson/job/ourproject/697/696/

Greetings, Daniel

[Juozas Šalna]

On Fri, 01 Dec 2006 16:30:01 +0200, <[hidden email]> wrote:

> Hello,
>
> when I click on "previous build" oder "next build" the buildnumber is  
> added to the url, responding in a 404. Here an example:
> I'm watching build 696 and want to go back to 695 the buildnumber is  
> added before the old buildnumber to the url:
> http://ourbuildserver/hudson/job/ourproject/695/696/
> goind forward is resulting in this url:
> http://ourbuildserver/hudson/job/ourproject/697/696/
>
> Greetings, Daniel
>

maybe your project name contains spaces?
which are converted to %20 ?


--
juozas šalna

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[daniel.galan]

Hi,

> > when I click on "previous build" oder "next build" the
> buildnumber is
> > added to the url, responding in a 404. Here an example:
> > I'm watching build 696 and want to go back to 695 the
> buildnumber is  
> > added before the old buildnumber to the url:
> > http://ourbuildserver/hudson/job/ourproject/695/696/
> > goind forward is resulting in this url:
> > http://ourbuildserver/hudson/job/ourproject/697/696/
> maybe your project name contains spaces?
> which are converted to %20 ?

That's true, isn't that possible?
Anyhow, it's a bug. Either you allow spaces in the project name, or you don't (by validating the user made in the prjectname field).
I think spaces should be supported, the way the link is generated for "previous/next build" should be fixed.

Thanks for the Workaround

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Vladimir Sizikov]

Hi Daniel,

Thanks for the report!
Indeed, this is a bug.

Can you file it in Hudson's IssueTracker:
https://hudson.dev.java.net/servlets/ProjectIssues

Thanks,
  --Vladimir

On Fri, Dec 01, 2006 at 05:27:09PM +0100, [hidden email] wrote:

> Hi,
> > > when I click on "previous build" oder "next build" the
> > buildnumber is
> > > added to the url, responding in a 404. Here an example:
> > > I'm watching build 696 and want to go back to 695 the
> > buildnumber is  
> > > added before the old buildnumber to the url:
> > > http://ourbuildserver/hudson/job/ourproject/695/696/
> > > goind forward is resulting in this url:
> > > http://ourbuildserver/hudson/job/ourproject/697/696/
> > maybe your project name contains spaces?
> > which are converted to %20 ?
> That's true, isn't that possible?
> Anyhow, it's a bug. Either you allow spaces in the project name, or you don't (by validating the user made in the prjectname field).
> I think spaces should be supported, the way the link is generated for "previous/next build" should be fixed.
>
> Thanks for the Workaround
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Vladimir Sizikov]

Hi,

It gets even more interesting.
Basically, ANY unsafe character in project name leads to the problems
with Previous/Next build links.

For example, any Russian letters or German ones (like u with two dots)
trigger the problem.

I have a rough patch that fixes the problem, but I'd prefer to wait
for Kohsuke to return and review it before posting it to the
repository.

Maybe a better fix would be to fix Stapler's Ancestor so that it
would always return properly quoted URL, rather than unquoted value
(currently, the behavior is unclear and is not specified in
javadocs).

Here the patch, just in case. I verified that it fixed the problem
with Prev/Next build for me.

Index: src/main/java/hudson/Functions.java
===================================================================
RCS file: /cvs/hudson/hudson/main/core/src/main/java/hudson/Functions.java,v
retrieving revision 1.4
diff -u -r1.4 Functions.java
--- src/main/java/hudson/Functions.java 24 Nov 2006 20:08:18 -0000 1.4
+++ src/main/java/hudson/Functions.java 1 Dec 2006 19:16:51 -0000
@@ -18,6 +18,8 @@
 import java.util.logging.LogRecord;
 import java.util.logging.SimpleFormatter;
 import java.io.File;
+import java.net.URI;
+import java.net.URISyntaxException;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -65,13 +67,23 @@
         return buf.toString();
     }
 
+    private static String quoteForUrl(String str) {
+        try {
+            URI uri = new URI(null, str, null);
+            return uri.toASCIIString();
+        } catch (URISyntaxException e) {
+            // never happens
+            throw new AssertionError();
+        }
+    }
+
     public static RunUrl decompose(StaplerRequest req) {
         @SuppressWarnings("unchecked") // pre-JDK 5 API?
         List<Ancestor> ancestors = req.getAncestors();
         for (Ancestor anc : ancestors) {
             if(anc.getObject() instanceof Run) {
                 // bingo
-                String ancUrl = anc.getUrl();
+                String ancUrl = quoteForUrl(anc.getUrl());
 
                 String reqUri = req.getOriginalRequestURI();
 
Thanks,
  --Vladimir

On Fri, Dec 01, 2006 at 09:05:14AM -0800, Vladimir Sizikov wrote:

> Hi Daniel,
>
> Thanks for the report!
> Indeed, this is a bug.
>
> Can you file it in Hudson's IssueTracker:
> https://hudson.dev.java.net/servlets/ProjectIssues
>
> Thanks,
>   --Vladimir
>
> On Fri, Dec 01, 2006 at 05:27:09PM +0100, [hidden email] wrote:
> > Hi,
> > > > when I click on "previous build" oder "next build" the
> > > buildnumber is
> > > > added to the url, responding in a 404. Here an example:
> > > > I'm watching build 696 and want to go back to 695 the
> > > buildnumber is  
> > > > added before the old buildnumber to the url:
> > > > http://ourbuildserver/hudson/job/ourproject/695/696/
> > > > goind forward is resulting in this url:
> > > > http://ourbuildserver/hudson/job/ourproject/697/696/
> > > maybe your project name contains spaces?
> > > which are converted to %20 ?
> > That's true, isn't that possible?
> > Anyhow, it's a bug. Either you allow spaces in the project name, or you don't (by validating the user made in the prjectname field).
> > I think spaces should be supported, the way the link is generated for "previous/next build" should be fixed.
> >
> > Thanks for the Workaround
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jesse Glick]

Vladimir Sizikov wrote:
> +        try {
> +            URI uri = new URI(null, str, null);
> +            return uri.toASCIIString();
> +        } catch (URISyntaxException e) {
> +            // never happens
> +            throw new AssertionError();
> +        }

Possibly simpler (try it):

return URI.create(str).toASCIIString();

-J.

--
[hidden email]  x22801  netbeans.org  ant.apache.org
       http://google.com/search?q=e%5E%28pi*i%29%2B1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Vladimir Sizikov]

Hi,

On Fri, Dec 01, 2006 at 03:17:21PM -0500, Jesse Glick wrote:

> Vladimir Sizikov wrote:
> >+        try {
> >+            URI uri = new URI(null, str, null);
> >+            return uri.toASCIIString();
> >+        } catch (URISyntaxException e) {
> >+            // never happens
> >+            throw new AssertionError();
> >+        }
>
> Possibly simpler (try it):
>
> return URI.create(str).toASCIIString();

Oh, I didn't know about this shortcut, but it doesn't work here,
since create() calls single arg constructor while I need 3-arg one,
since single-arg constructor doesn't like unquoted illegal chars:

"The single-argument constructor requires any illegal characters in
its argument to be quoted"

Thanks,
  --Vladimir

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Wolfram Kroll-2]

Am 01.12.2006 um 17:27 schrieb <[hidden email]>:

> Hi,
>>> when I click on "previous build" oder "next build" the
>> buildnumber is
>>> added to the url, responding in a 404. Here an example:
>>> I'm watching build 696 and want to go back to 695 the
>> buildnumber is
>>> added before the old buildnumber to the url:
>>> http://ourbuildserver/hudson/job/ourproject/695/696/
>>> goind forward is resulting in this url:
>>> http://ourbuildserver/hudson/job/ourproject/697/696/
>> maybe your project name contains spaces?
>> which are converted to %20 ?
> That's true, isn't that possible?
> Anyhow, it's a bug. Either you allow spaces in the project name, or
> you don't (by validating the user made in the prjectname field).
> I think spaces should be supported, the way the link is generated for
> "previous/next build" should be fixed.

Spaces in names also lead to problems when using build slaves. The
project name is not quoted on the command line which is used to drive
the build on the remote machine. I hope I remember correctly my first
day with hudson... From then on I avoided spaces in job names.

Wolfram

>
> Thanks for the Workaround
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Vladimir Sizikov]

Hi,

I filed ISSUE #186 for this:
https://hudson.dev.java.net/issues/show_bug.cgi?id=186
"Invalid Prev/Next build links in projects with spaces in their names"

Kohsuke, would you be able to review the fix (quoted below)?

Thanks,
  --Vladimir

On Fri, Dec 01, 2006 at 11:26:15AM -0800, Vladimir Sizikov wrote:

> Hi,
>
> It gets even more interesting.
> Basically, ANY unsafe character in project name leads to the problems
> with Previous/Next build links.
>
> For example, any Russian letters or German ones (like u with two dots)
> trigger the problem.
>
> I have a rough patch that fixes the problem, but I'd prefer to wait
> for Kohsuke to return and review it before posting it to the
> repository.
>
> Maybe a better fix would be to fix Stapler's Ancestor so that it
> would always return properly quoted URL, rather than unquoted value
> (currently, the behavior is unclear and is not specified in
> javadocs).
>
> Here the patch, just in case. I verified that it fixed the problem
> with Prev/Next build for me.
>
> Index: src/main/java/hudson/Functions.java
> ===================================================================
> RCS file: /cvs/hudson/hudson/main/core/src/main/java/hudson/Functions.java,v
> retrieving revision 1.4
> diff -u -r1.4 Functions.java
> --- src/main/java/hudson/Functions.java 24 Nov 2006 20:08:18 -0000 1.4
> +++ src/main/java/hudson/Functions.java 1 Dec 2006 19:16:51 -0000
> @@ -18,6 +18,8 @@
>  import java.util.logging.LogRecord;
>  import java.util.logging.SimpleFormatter;
>  import java.io.File;
> +import java.net.URI;
> +import java.net.URISyntaxException;
>  
>  /**
>   * @author Kohsuke Kawaguchi
> @@ -65,13 +67,23 @@
>          return buf.toString();
>      }
>  
> +    private static String quoteForUrl(String str) {
> +        try {
> +            URI uri = new URI(null, str, null);
> +            return uri.toASCIIString();
> +        } catch (URISyntaxException e) {
> +            // never happens
> +            throw new AssertionError();
> +        }
> +    }
> +
>      public static RunUrl decompose(StaplerRequest req) {
>          @SuppressWarnings("unchecked") // pre-JDK 5 API?
>          List<Ancestor> ancestors = req.getAncestors();
>          for (Ancestor anc : ancestors) {
>              if(anc.getObject() instanceof Run) {
>                  // bingo
> -                String ancUrl = anc.getUrl();
> +                String ancUrl = quoteForUrl(anc.getUrl());
>  
>                  String reqUri = req.getOriginalRequestURI();
>  
> Thanks,
>   --Vladimir
>
> On Fri, Dec 01, 2006 at 09:05:14AM -0800, Vladimir Sizikov wrote:
> > Hi Daniel,
> >
> > Thanks for the report!
> > Indeed, this is a bug.
> >
> > Can you file it in Hudson's IssueTracker:
> > https://hudson.dev.java.net/servlets/ProjectIssues
> >
> > Thanks,
> >   --Vladimir
> >
> > On Fri, Dec 01, 2006 at 05:27:09PM +0100, [hidden email] wrote:
> > > Hi,
> > > > > when I click on "previous build" oder "next build" the
> > > > buildnumber is
> > > > > added to the url, responding in a 404. Here an example:
> > > > > I'm watching build 696 and want to go back to 695 the
> > > > buildnumber is  
> > > > > added before the old buildnumber to the url:
> > > > > http://ourbuildserver/hudson/job/ourproject/695/696/
> > > > > goind forward is resulting in this url:
> > > > > http://ourbuildserver/hudson/job/ourproject/697/696/
> > > > maybe your project name contains spaces?
> > > > which are converted to %20 ?
> > > That's true, isn't that possible?
> > > Anyhow, it's a bug. Either you allow spaces in the project name, or you don't (by validating the user made in the prjectname field).
> > > I think spaces should be supported, the way the link is generated for "previous/next build" should be fixed.
> > >
> > > Thanks for the Workaround
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Kohsuke Kawaguchi-2]

Thanks. I do have a few jobs with whitespaces in it and I have one job
with all Japanese characters in the test environment, but I failed to
notice. I blame i18n handling in servlets and not my carelessness :-)

I think you are right that we should probably fix this in stapler, so
I'll adapt your change for stapler.

I just deployed 1.65-RC on my production system for final verification,
so let's apply this first thing after 1.65 release (which should be
tomorrow evening.)


Vladimir Sizikov wrote:

> Hi,
>
> I filed ISSUE #186 for this:
> https://hudson.dev.java.net/issues/show_bug.cgi?id=186
> "Invalid Prev/Next build links in projects with spaces in their names"
>
> Kohsuke, would you be able to review the fix (quoted below)?
>
> Thanks,
>   --Vladimir
>
> On Fri, Dec 01, 2006 at 11:26:15AM -0800, Vladimir Sizikov wrote:
>> Hi,
>>
>> It gets even more interesting.
>> Basically, ANY unsafe character in project name leads to the problems
>> with Previous/Next build links.
>>
>> For example, any Russian letters or German ones (like u with two dots)
>> trigger the problem.
>>
>> I have a rough patch that fixes the problem, but I'd prefer to wait
>> for Kohsuke to return and review it before posting it to the
>> repository.
>>
>> Maybe a better fix would be to fix Stapler's Ancestor so that it
>> would always return properly quoted URL, rather than unquoted value
>> (currently, the behavior is unclear and is not specified in
>> javadocs).
>>
>> Here the patch, just in case. I verified that it fixed the problem
>> with Prev/Next build for me.
>>
>> Index: src/main/java/hudson/Functions.java
>> ===================================================================
>> RCS file: /cvs/hudson/hudson/main/core/src/main/java/hudson/Functions.java,v
>> retrieving revision 1.4
>> diff -u -r1.4 Functions.java
>> --- src/main/java/hudson/Functions.java 24 Nov 2006 20:08:18 -0000 1.4
>> +++ src/main/java/hudson/Functions.java 1 Dec 2006 19:16:51 -0000
>> @@ -18,6 +18,8 @@
>>  import java.util.logging.LogRecord;
>>  import java.util.logging.SimpleFormatter;
>>  import java.io.File;
>> +import java.net.URI;
>> +import java.net.URISyntaxException;
>>  
>>  /**
>>   * @author Kohsuke Kawaguchi
>> @@ -65,13 +67,23 @@
>>          return buf.toString();
>>      }
>>  
>> +    private static String quoteForUrl(String str) {
>> +        try {
>> +            URI uri = new URI(null, str, null);
>> +            return uri.toASCIIString();
>> +        } catch (URISyntaxException e) {
>> +            // never happens
>> +            throw new AssertionError();
>> +        }
>> +    }
>> +
>>      public static RunUrl decompose(StaplerRequest req) {
>>          @SuppressWarnings("unchecked") // pre-JDK 5 API?
>>          List<Ancestor> ancestors = req.getAncestors();
>>          for (Ancestor anc : ancestors) {
>>              if(anc.getObject() instanceof Run) {
>>                  // bingo
>> -                String ancUrl = anc.getUrl();
>> +                String ancUrl = quoteForUrl(anc.getUrl());
>>  
>>                  String reqUri = req.getOriginalRequestURI();
>>  
>> Thanks,
>>   --Vladimir
>>
>> On Fri, Dec 01, 2006 at 09:05:14AM -0800, Vladimir Sizikov wrote:
>> > Hi Daniel,
>> >
>> > Thanks for the report!
>> > Indeed, this is a bug.
>> >
>> > Can you file it in Hudson's IssueTracker:
>> > https://hudson.dev.java.net/servlets/ProjectIssues
>> >
>> > Thanks,
>> >   --Vladimir
>> >
>> > On Fri, Dec 01, 2006 at 05:27:09PM +0100, [hidden email] wrote:
>> > > Hi,
>> > > > > when I click on "previous build" oder "next build" the
>> > > > buildnumber is
>> > > > > added to the url, responding in a 404. Here an example:
>> > > > > I'm watching build 696 and want to go back to 695 the
>> > > > buildnumber is  
>> > > > > added before the old buildnumber to the url:
>> > > > > http://ourbuildserver/hudson/job/ourproject/695/696/
>> > > > > goind forward is resulting in this url:
>> > > > > http://ourbuildserver/hudson/job/ourproject/697/696/
>> > > > maybe your project name contains spaces?
>> > > > which are converted to %20 ?
>> > > That's true, isn't that possible?
>> > > Anyhow, it's a bug. Either you allow spaces in the project name, or you don't (by validating the user made in the prjectname field).
>> > > I think spaces should be supported, the way the link is generated for "previous/next build" should be fixed.
>> > >
>> > > Thanks for the Workaround

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]