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

Failed writing body on Windows #3175

Closed
bagder opened this issue Oct 25, 2018 · 14 comments

Comments

Projects
None yet
6 participants
@bagder
Copy link
Member

commented Oct 25, 2018

For this command curl https://www.vg.no, I'm always getting a

Failed writing body (16366 != 16384). The sizes can differ.
Options -N, -B does not help. Writing to a file is no problem.

Originally posted by @gvanem in #3161 (comment)

@bagder

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2018

@bitcrazed; I presume you don't see this?

@gvanem: what compiler/build did you use when you get this?

@gvanem

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

what compiler/build did you use when you get this?

MSVC-2017 (both libcurl.dll and static). I also tried with MinGW (gcc 5.1). It also happens there.
But more with something like:

Failed writing body (1399 != 1400)

smaller chunks and differences. But it depends on URL.

Maybe other Windows users can test a command like:

curl -vL https://www.vg.no
@bitcrazed

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

@bagder Not seen this before. Will take a look this weekend.

@bitcrazed

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

@bagder - Just confirmed that I don't see the error you describe above in the curl shipped in-box in the Win10 1809 build, though I do see the following appended to the end of curl -vL https://www.vg.no:

* Connection #0 to host www.vg.no left intact:

image

@gvanem

This comment has been minimized.

Copy link
Member

commented Nov 2, 2018

Connection #0 to host www.vg.no left intact:

It's irrelevant AFAICS. I fixed it by either:

  • disable the call to configure_terminal().
  • merging #3212.

The problem is also gone either way. So I'll assume it's the fault of current configure_terminal().

BTW, how does this call know if VT output has any effect. It seems to ass-u-me CMD is used. I use 4NT as my shell.

@bitcrazed

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

@gvanem Looks like the Connection #0 to host www.vg.no left intact: message is related to forcing the codepage to 65001. @mattn's approach in #3212 is a better & more complete solution that also removes the forced switch to 65001.

Currently, the only way to know whether the call to SetConsoleMode(…ENABLE_VIRTUAL_TERMINAL_PROCESSING) actually works is to subsequently call GetConsoleMode(…) and test if the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode flag is still set. In versions of Windows 10 < 1809, the flag can be set but will be ignored and won't 'stick'.

@bagder bagder closed this in 5bfaa86 Nov 5, 2018

@gvanem

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

I still get the curl: (23) Failed writing body (0 != 3942) with curl -o NUL ...

Edit:

  • Further tracing shows WriteConsole() in this case fails with GetLastError() == INVALID_FUNCTION.
    This link explains it better (although for stdin).

@jay jay reopened this Nov 20, 2018

@gvanem

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

I don't follow all the OutStruct members, but testing for:

  if (!outs->s_isreg && !outs->fopened) {

instead is maybe better?

@binkley

This comment has been minimized.

Copy link

commented Dec 23, 2018

What I observe on the curl shipped with Git for Windows:

$ curl -X POST -NsS http://localhost:8080/actuator/shutdown >/dev/null
curl: (23) Failed writing body (0 != 35)

I've tried several variations (-o /dev/null), etc. Perhaps this is a problem with msys/mingw?
Version:

$ uname -a
MINGW64_NT-10.0 BREE 2.11.2(0.329/5/3) 2018-11-10 14:38 x86_64 Msys
@gvanem

This comment has been minimized.

Copy link
Member

commented Dec 23, 2018

@binkley
You mean from this: https://git-scm.com/download/win.
The Portable version there has a curl version of 7.63.0 which exhibit that same Failed writing body error.

@opav

This comment has been minimized.

Copy link

commented Feb 22, 2019

Version 7.64.0 still failing with same Failed writing body error with command curl -o NUL, same for 7.63 and 7.62.

Only way to get curl working for me is to fallback to a version 7.61.0 from https://dl.uxnr.de/build/curl/curl_winssl_msys2_mingw32_stc/curl-7.61.0/curl-7.61.0.zip

Hopefully curl authors can fix it in next version.

@jay

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

Version 7.64.0 still failing with same Failed writing body error

Yes you are right I forgot this wasn't fully solved. Try this:

diff --git a/src/tool_cb_wrt.c b/src/tool_cb_wrt.c
index 195d6e7..1944f16 100644
--- a/src/tool_cb_wrt.c
+++ b/src/tool_cb_wrt.c
@@ -79,6 +79,9 @@ size_t tool_write_cb(char *buffer, size_t sz, size_t nmemb, void *userdata)
   struct OperationConfig *config = outs->config;
   size_t bytes = sz * nmemb;
   bool is_tty = config->global->isatty;
+#ifdef WIN32
+  CONSOLE_SCREEN_BUFFER_INFO console_info;
+#endif
 
   /*
    * Once that libcurl has called back tool_write_cb() the returned value
@@ -156,7 +159,9 @@ size_t tool_write_cb(char *buffer, size_t sz, size_t nmemb, void *userdata)
   }
 
 #ifdef _WIN32
-  if(isatty(fileno(outs->stream))) {
+  if(isatty(fileno(outs->stream)) &&
+     GetConsoleScreenBufferInfo(
+       (HANDLE)_get_osfhandle(fileno(outs->stream)), &console_info)) {
     DWORD in_len = (DWORD)(sz * nmemb);
     wchar_t* wc_buf;
     DWORD wc_len;

I think we should probably use is_tty which is just config->global->isatty but it looks like that's dependent on the progress meter logic. Anyway that may be a better way to tackle it

curl/src/tool_operate.c

Lines 747 to 757 in f3294d9

if(output_expected(this_url, uploadfile) && outs.stream &&
isatty(fileno(outs.stream)))
/* we send the output to a tty, therefore we switch off the progress
meter */
global->noprogress = global->isatty = TRUE;
else {
/* progress meter is per download, so restore config
values */
global->noprogress = orig_noprogress;
global->isatty = orig_isatty;
}

@bagder bagder added help wanted and removed help wanted labels Feb 22, 2019

@bagder

This comment has been minimized.

Copy link
Member Author

commented Feb 22, 2019

funny that I added that label exactly while you were writing your comment =)

I think we should probably use is_tty which is just config->global->isatty but it looks like that's dependent on the progress meter logic

It doesn't have to be though.

jay added a commit that referenced this issue Mar 26, 2019

tool_cb_wrt: fix writing to Windows null device NUL
- Improve console detection.

Prior to this change WriteConsole could be called to write to a handle
that may not be a console, which would cause an error. This issue is
limited to character devices that are not also consoles such as the null
device NUL.

Bug: #3175 (comment)
Reported-by: Gisle Vanem
@jay

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

With consideration of the next release I used my original patch rather than rewrite global->isatty logic. If the same console check is needed elsewhere we can revisit it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.