Skip to content

Commit

Permalink
Issue #180: don't write body if HTTP method is DELETE.
Browse files Browse the repository at this point in the history
  • Loading branch information
@kohsuke
kohsuke committed Apr 26, 2015
1 parent fd43429 commit b976e0e
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/main/java/org/kohsuke/github/Requester.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ public String getResponseHeader(String header) {
* Set up the request parameters or POST payload.
*/
private void buildRequest() throws IOException {
if (!method.equals("GET")) {
if (isMethodWithBody()) {
uc.setDoOutput(true);
uc.setRequestProperty("Content-type", contentType);

Expand All @@ -296,7 +296,6 @@ private void buildRequest() throws IOException {
for (Entry e : args) {
json.put(e.key, e.value);
}
MAPPER.writeValue(uc.getOutputStream(), json);

This comment has been minimized.

Sign in to view

Copy link
@henryju

henryju Apr 27, 2015

Contributor

Seems this change is breaking creation of pull requests (and probably other things) since json Map is no more used.

This comment has been minimized.

Sign in to view

Copy link
@kohsuke

kohsuke Apr 27, 2015

Author Collaborator

I haven't released this change. How is it that this is breaking stuff?

Do you have stack trace?

This comment has been minimized.

Sign in to view

Copy link
@henryju

henryju Apr 27, 2015

Contributor

I didn't say it was a regression in last release. But witth latest SNAPSHOT I was unable to create a pull request (HTTP error 400, invalid json body or something like that). Just try to run test PullRequestTest#createPullRequest to reproduce. Sorry if I was not clear.

This comment has been minimized.

Sign in to view

Copy link
@if6was9

if6was9 May 9, 2015

I can confirm that 1.69-SNAPSHOT is badly broken.

For GHRepository.edit() operations, which use POST (aren't they supposed to use PATCH), a 1.68 request looks like this:

POST /repos/if6was9/openrtb HTTP/1.1
Authorization: Basic XXXXXXX
Accept-Encoding: gzip
Content-type: application/x-www-form-urlencoded
User-Agent: Java/1.7.0_75
Host: api.github.com
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Connection: keep-alive
Content-Length: 75

{"description":"description Sat May 09 00:33:09 PDT 2015","name":"openrtb"}

in 1.69-SNAPSHOT, it sends the request as a GET, which doesn't fail, but doesn't do anything at all.

GET /repos/if6was9/openrtb HTTP/1.1
Authorization: Basic XXXXXX
Accept-Encoding: gzip
Content-type: application/x-www-form-urlencoded
User-Agent: Java/1.7.0_75
Host: api.github.com
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Connection: keep-alive

An unrelated issue is that github-api is setting the Content-type and Accept headers incorrectly. GitHub seems to tolerate it, but it is not correct.

This comment has been minimized.

Sign in to view

Copy link
@lanwen

lanwen Jun 15, 2015

Contributor

the main problem fixed in #200
} else {
try {
byte[] bytes = new byte[32768];
Expand All @@ -311,6 +310,12 @@ private void buildRequest() throws IOException {
}
}

private boolean isMethodWithBody() {
if (method.equals("GET")) return false;
if (method.equals("DELETE")) return false;
return true;
}

/**
* Loads pagenated resources.
*
Expand Down

0 comments on commit b976e0e

Please sign in to comment.