Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Charset variable includes a Content-Type attribute after "charset" #718

Closed
vaabr opened this issue Feb 25, 2020 · 0 comments · Fixed by #751 or #754
Closed

Charset variable includes a Content-Type attribute after "charset" #718

vaabr opened this issue Feb 25, 2020 · 0 comments · Fixed by #751 or #754

Comments

@vaabr
Copy link

vaabr commented Feb 25, 2020

Bug Report

Description

Charset variable in com.github.kittinunf.fuel.core.BodyRepresentation.kt includes an attribute after the charset which invokes java.nio.charset.IllegalCharsetNameException: UTF-8; API-VERSION=5.1. Azure DevOps sends this as the Content-Type: application/json; charset=utf-8; api-version=5.1.

To Reproduce

Steps to reproduce the behavior:

  1. Send a post request to Azure DevOps
  2. Get the response
  3. Observe the exception being thrown
fun foo(body: String, auth: Pair<String, String>) = Fuel.post("https://dev.azure.com/*organization*/*project*/_apis/wit/workitems/\$Bug", listOf("api-version" to "5.1", "validateOnly" to "true"))
            .authentication()
            .basic(auth.first, auth.second)
            .header("Content-Type" to "application/json-patch+json")
            .body(body)
            .response()

Expected behavior

TEXT_CONTENT_TYPE regex excludes attributes after the charset

Environment

Development Machine

  • OS: Win10
  • IDE: IntelliJ
  • Fuel version: 2.2.1
  • Kotlin version: 1.3.61

Additional context

java.nio.charset.IllegalCharsetNameException: UTF-8; API-VERSION=5.1
	at java.base/java.nio.charset.Charset.checkName(Charset.java:308)
	at java.base/java.nio.charset.Charset.lookup2(Charset.java:482)
	at java.base/java.nio.charset.Charset.lookup(Charset.java:462)
	at java.base/java.nio.charset.Charset.forName(Charset.java:526)
	at com.github.kittinunf.fuel.core.BodyRepresentationKt.representationOfBytes(BodyRepresentation.kt:23)
	at com.github.kittinunf.fuel.core.requests.DefaultBody.asString(DefaultBody.kt:30)
	at com.github.kittinunf.fuel.core.Response.toString(Response.kt:50)
	at java.base/java.lang.String.valueOf(String.java:2951)
	at java.base/java.lang.StringBuilder.append(StringBuilder.java:168)

ADO reference:
https://docs.microsoft.com/en-us/rest/api/azure/devops/wit/work%20items/create?view=azure-devops-rest-5.1

UPD:
I have found a possible solution:
In com.github.kittinunf.fuel.core.BodyRepresentation.kt in the string 22 replace

val charsetName = charsetGroup.substringAfter('=').toUpperCase()

with

val charsetName = charsetGroup.substringAfter('=').substringBefore(';').toUpperCase()

Since the second group is ; charset=utf-8; api-version=5.1 it substrings the charset attribute value.

vaabr added a commit to vaabr/fuel that referenced this issue Jun 5, 2020
Added substringBefore to exclude any parameters after the charset parameter and fix the issue.
@vaabr vaabr mentioned this issue Jun 5, 2020
12 tasks
vaabr added a commit to vaabr/fuel that referenced this issue Jun 5, 2020
Added unit test that replicates the conditions kittinunf#718 was encountered under and verifies the fix
kittinunf pushed a commit that referenced this issue Jun 6, 2020
vaabr added a commit to vaabr/fuel that referenced this issue Jun 6, 2020
As per SleeplessByte comment, my previous changes did not fix the root of the kittinunf#718 issue and it still occured if the order of parameters was different. In this commit I have fixed the issue, added unit tests to prove that and changed the default encoding to ASCII
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant