gitblit-org / gitblit

pure java git solution
http://gitblit.com
Apache License 2.0
2.28k stars 670 forks source link

Unable to browse log for any repository #320

Closed gitblit closed 9 years ago

gitblit commented 9 years ago

Originally reported on Google Code with ID 24

INFO  Found more URL path parts then expected, these will be ignored. Url: 'http://192.168.41.84:8445/gitblit/log/gitblit-crowd.git/refs/heads/master',
mountpath: 'log', urlPath: 'gitblit-crowd.git/refs/heads/master', expected 2 parameters
was 4
INFO  Loading properties files from jar:file:/home/pf/gitblit/gitblit.jar!/com/gitblit/wicket/GitBlitWebApp.properties
INFO  Loading properties files from jar:file:/home/pf/gitblit/ext/wicket-1.4.18.jar!/org/apache/wicket/Application.properties
ERROR /home/pf/gitblit/git/gitblit-crowd.git failed to get refs revlog for path null
java.lang.NullPointerException
    at org.eclipse.jgit.lib.ObjectIdOwnerMap.get(ObjectIdOwnerMap.java:131)
    at org.eclipse.jgit.revwalk.RevWalk.parseAny(RevWalk.java:809)
    at org.eclipse.jgit.revwalk.RevWalk.parseCommit(RevWalk.java:724)
    at com.gitblit.utils.JGitUtils.getRevLog(JGitUtils.java:893)
    at com.gitblit.utils.JGitUtils.getRevLog(JGitUtils.java:854)
    at com.gitblit.wicket.panels.LogPanel.<init>(LogPanel.java:65)
    at com.gitblit.wicket.pages.LogPage.<init>(LogPage.java:34)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
    at org.apache.wicket.session.DefaultPageFactory.createPage(DefaultPageFactory.java:188)
    at org.apache.wicket.session.DefaultPageFactory.newPage(DefaultPageFactory.java:89)
    at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.newPage(BookmarkablePageRequestTarget.java:305)
    at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.getPage(BookmarkablePageRequestTarget.java:320)
    at org.apache.wicket.request.target.component.BookmarkablePageRequestTarget.processEvents(BookmarkablePageRequestTarget.java:234)
    at org.apache.wicket.request.AbstractRequestCycleProcessor.processEvents(AbstractRequestCycleProcessor.java:92)
    at org.apache.wicket.RequestCycle.processEventsAndRespond(RequestCycle.java:1252)
    at org.apache.wicket.RequestCycle.step(RequestCycle.java:1331)
    at org.apache.wicket.RequestCycle.steps(RequestCycle.java:1438)
    at org.apache.wicket.RequestCycle.request(RequestCycle.java:546)
    at org.apache.wicket.protocol.http.WicketFilter.doGet(WicketFilter.java:486)
    at org.apache.wicket.protocol.http.WicketFilter.doFilter(WicketFilter.java:319)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1323)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:476)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:119)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:517)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:225)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:937)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:406)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:183)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:871)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:117)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:110)
    at org.eclipse.jetty.server.Server.handle(Server.java:346)
    at org.eclipse.jetty.server.HttpConnection.handleRequest(HttpConnection.java:589)
    at org.eclipse.jetty.server.HttpConnection$RequestHandler.headerComplete(HttpConnection.java:1048)
    at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:601)
    at org.eclipse.jetty.http.HttpParser.parseAvailable(HttpParser.java:214)
    at org.eclipse.jetty.server.HttpConnection.handle(HttpConnection.java:411)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:535)
    at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:40)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:529)
    at java.lang.Thread.run(Thread.java:662)

Reported by trygvis on 2011-10-10 06:29:27

gitblit commented 9 years ago
For now set this property in gitblit.properties:

web.mountParameters=false

Reported by James.Moger on 2011-10-10 12:05:02

gitblit commented 9 years ago
I'd like you to do 2 things for me:

1. Using your browser, inspect the link that you click to generate this error and paste
the <a> tag on this issue.

2. Tell me what browser you are using.  I think you said you were running latest Ubuntu.

This issue has come up before and it has to do with forward slashes and how they are
encoded/decoded by Wicket but I can not completely eliminate the browser from the equation
either.

Using parameterized urls instead of mounted urls (web.mountParameters=false) solves
your immediate problem.

Reported by James.Moger on 2011-10-10 12:50:47

gitblit commented 9 years ago
The link from the address bar is 

  http://192.168.41.84/gitblit/log/gitblit-crowd.git/refs%2Fheads%2Fmaster

The generated HTML is:

  <a href="../log/gitblit-crowd.git/refs%2Fheads%2Fmaster" class="list name"><span>master</span></a>

Note that I'm using this behind mod_proxy so a local httpd is accessing this as http://127.0.0.1:8445/gitblit.
To get this far I also needed another patch, see this commit in my repository: https://github.com/trygvis/gitblit/commit/3ab5b06fa234c7ee802757562355d39976a811fa.
Sorry for not noting that in the first issue.

Reported by trygvis on 2011-10-10 13:39:57

gitblit commented 9 years ago
The reason the request fails is because the request is formatted as:
   gitblit-crowd.git/refs/heads/master
instead of:
   gitblit-crowd.git/refs%2Fheads%2Fmaster

Wicket splits the request by '/' which results in the wrong number of parameters, 4
-- first request format -- instead of 2.

Workarounds/solutions:
1. Fix: Don't mount urls (web.mountParameters = false)
2. Fix/Hack: Don't use '/' within a parameter (web.forwardSlashCharacter = !)
3. mod_proxy allowEncodedSlashes
http://httpd.apache.org/docs/2.2/mod/core.html#allowencodedslashes

I'm interested in knowing the results of #3.  I think you want the NoDecode option.

I'd also like to know more about the context path change.  Mod proxy would not proxy
to the root of GO?

Reported by James.Moger on 2011-10-10 14:25:52

gitblit commented 9 years ago
I can try the mod_proxy fixes at work tomorror. web.mountParameters = false works around
the problem, but I do like the pretty URLs.

I've used urlrewritefilter [1] with luck previously to support pretty URLs. I haven't
ever used wicket so I don't know if Wicket has something better built in.

The reason for the context path change is because I'm using plain rewriting and not
mod_ajp. I want gitblit to be exposed as http://host/gitblit and to get gitblit to
have sane URLs it needed to be deployed under the same context. If didn't it would
create urls that pointed to http://host/git/gitblit-crowd.git (note the missing /gitblit).

[1]: http://code.google.com/p/urlrewritefilter/?

Reported by trygvis on 2011-10-10 18:49:18

gitblit commented 9 years ago
So.... hows it going?

Reported by James.Moger on 2011-10-12 11:54:51

gitblit commented 9 years ago
NoDecode is not available until 2.2.18 and Ubuntu only has 2.2.17.

As a side note, adding "ProxyPreserveHost On" had the nice effect of fixing the git
clone URLs.

Reported by trygvis on 2011-10-12 14:14:15

gitblit commented 9 years ago
I just realized that Ubuntu is releasing 11.10 today so I might get to try this out
quite soon after all.

Reported by trygvis on 2011-10-13 10:06:55

gitblit commented 9 years ago
So how is the url rewriting going?

Reported by James.Moger on 2011-11-08 20:21:00

gitblit commented 9 years ago
I'm getting URLs like this:

  http://192.168.41.84/gitblit/blobdiff/gitblit-crowd.git/9b72a2e674f7cbc320b0b72fc71ad813d296ab12/src%2Fcom%2Fgitblit%2FLauncher.java

which hits the wicket code (after I set "AllowEncodedSlashes NoDecode").

However there's no content on the page, and the path shown in the display is "[gitblit-crowd.git]
/ src%2Fcom%2Fgitblit%2FLauncher.java" which leads me to think that the wicket code
is not decoding the request properly.

Reported by trygvis on 2011-11-09 08:49:06

gitblit commented 9 years ago
Hmmm.  Clearly something is not working quite right - the displayed path should be what
you expect, not the %2F encoded value.

Your url is properly formatted and I confirm that it works here (after switching repo
names).  Is this the url you get after rewrite and that url is sent to Gitblit?  Is
it possible that the url is being double encoded?  I've seen that happen to; something
like src%202Fcom%202Fgitblit%202FLauncher.java

Reported by James.Moger on 2011-11-09 13:14:42

gitblit commented 9 years ago
While I prefer to fix this by making sure that the url getting to Gitblit is 100% correct,
I think we can try a workaround too.

There are a few page parameters that may have embedded forward slashes:
  r (repository parameter)
  h (object id or ref)
  f (path parameter)

Each of those parameters is retrieved from the Wicket PageParameters object by a static
function call in com.gitblit.wicket.WicketUtils.  i.e. getRepository, getObject, getPath.
 You could hack these methods to replace("%2F", "/") and report back how that works
for you.

I'm overdue on a new release by ~ 2 weeks.  I plan to release this week.  And then
again in late December or early January with JGit 1.2 amongst other things.

Reported by James.Moger on 2011-11-09 15:20:27

gitblit commented 9 years ago
This was fixed in v0.7.0 and/or worked-around by using one of the URL control settings.

Reported by James.Moger on 2012-01-11 21:47:07

steveno commented 9 years ago

Should this issue be closed?

gitblit commented 9 years ago

Hmm. Looks like the GoogleCode import didn't close all issues correctly. Doh!

steveno commented 9 years ago

:+1:

In that case you should close #314 and #310 too.

gitblit commented 9 years ago

Thanks. Done.