Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 367344

Summary:	[Server] We shouldn't use header to transport user requested data
Product:	[ECD] Orion	Reporter:	Malgorzata Janczarska <malgorzata.tomczyk>
Component:	Git	Assignee:	Tomasz Zarna <tomasz.zarna>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P5	CC:	john.arthorne, Szymon.Brandys
Version:	unspecified
Target Milestone:	0.5 M2
Hardware:	PC
OS:	Windows 7
Whiteboard:

Description Malgorzata Janczarska

2011-12-21 10:52:06 EST

in gitClient#getLog we are doing POST on HeadLocation or CommitLocation to get the location where we can find a git log.
The location is returned in response header, this is bad because header are not meant to transform business data. We should use response body instead.

Comment 1 John Arthorne

2011-12-21 11:52:43 EST

I don't know the details in this case, but the location of a resource seems like reasonable information to have in a header. A typical POST will include Location header in the response which is the URL of the resource that was created. If this is what's going on here it sounds reasonable. If we are inventing new header names like "Git-Location:" then yes it should probably be in the body instead.

Comment 2 Tomasz Zarna

2011-12-21 12:35:12 EST

I guess Gosia is referring to org.eclipse.orion.server.git.servlets.GitCommitHandlerV1.createCommitLocation(HttpServletRequest, HttpServletResponse, Repository, String) which purpose is to return a location for a log between two commits, so the user doesn't handcraft it. The name used there is "Location". This looks fine to me.

Comment 3 John Arthorne

2011-12-21 13:53:21 EST

It looks fine to me too. The method name is a bit confusing though. It is not "creating a location".  In HTTP terms, this is a POST request to create a new "log" resource, and the POST response contains the location (URI) of that new resource in the Location header. See HTTP spec 14.30:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

Comment 4 Szymon Brandys

2012-01-16 04:27:50 EST

Renaming the method to createCommitLogLocation or similar looks ok.

Comment 5 Malgorzata Janczarska

2012-01-27 09:12:15 EST

There is not sense to keep it open if we agree that this is a good pattern.

Comment 6 Szymon Brandys

2012-04-19 10:57:54 EDT

We need to reopen the discussion.

Of course it is ok to return Location of the new resource in the header. However response headers are very often lost e.g. if we call a method via a plug-in or if a call is converted to a task. If we could move "Location' from the header to the body, or at least have it in both places it would help.

Comment 7 Szymon Brandys

2012-04-19 10:58:28 EDT

See comment 6.

Comment 8 Tomasz Zarna

2012-05-04 05:49:58 EDT

Fixed with 6f444f382e59cb1c5e1be443c3504cc9bd6c6bf5