search

dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

Add option to add multipart charset #522

Closed saitouena closed 4 years ago

saitouena commented 4 years ago

related issue: https://github.com/dakrone/clj-http/issues/472

To handle non-ascii characters in a multipart filename, we need a way to set .setCharset and .setMode BROWSER_COMPATIBLE.

see also: https://stackoverflow.com/questions/3393445/international-characters-in-filename-in-mutipart-formdata https://qiita.com/otomoringo/items/f523c5ece1ef8471d149#修正箇所

I'm not sure if .setMode STRICT and .setCharset is used together. I didn't add validation for now.

dakrone commented 4 years ago

@saitouena thanks for adding this! Can you add a blurb in the documentation about the multipart-charset setting (maybe with an example of using it)?

saitouena commented 4 years ago

@saitouena thanks for adding this! Can you add a blurb in the documentation about the multipart-charset setting (maybe with an example of using it)?

added.

dakrone commented 4 years ago

Thanks @saitouena!

saitouena commented 4 years ago

@dakrone I appreciate your review. Do you have a plan for next release?

dakrone commented 4 years ago

Hi @saitouena I haven't planned for it, do you need this for something pretty soon?

saitouena commented 4 years ago

@dakrone We use clj-http on production (https://github.com/toyokumo/kintone-clj/pull/3#issue-331321290). This PR fixes some problem. I appreciate sooner release. If you are too busy to release, I try SNAPSHOT version.

dakrone commented 4 years ago

@saitouena okay, I'd like to do a release, but I need to fix a test failure on the 3.x branch first. Once that is fixed I'll do a release.

saitouena commented 4 years ago

@dakrone

but I need to fix a test failure on the 3.x branch first.

Which test do you mean by "a test failure on the 3.x branch"? All tests seem to pass. https://travis-ci.org/dakrone/clj-http/builds/608813523?utm_source=github_status&utm_medium=notification Do you mean this issue? https://github.com/dakrone/clj-http/pull/511#issuecomment-519310696

dakrone commented 4 years ago

@saitouena no, there is a separate failure in the gzip stream tests when running lein test :all that I need to fix.