Skip to content

Allow for POSTs that are larger than a few 100 bytes. #2528

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

Closed
wants to merge 2 commits into from

Conversation

dirkx
Copy link

@dirkx dirkx commented Sep 16, 2016

When sending POST responses of well over a K - _write() may not sent it all. Make sure we do -- but cap the individual writes - as somehow large 2-3k blobs seem to cause instability (on my 12F units).

@codecov-io
Copy link

codecov-io commented Sep 16, 2016

Current coverage is 27.80% (diff: 100%)

Merging #2528 into master will not change coverage

@@             master      #2528   diff @@
==========================================
  Files            20         20          
  Lines          3625       3625          
  Methods         335        335          
  Messages          0          0          
  Branches        656        656          
==========================================
  Hits           1008       1008          
  Misses         2441       2441          
  Partials        176        176          

Powered by Codecov. Last update 4897e00...268f200

else
if (written == 0)
return returnError(HTTPC_ERROR_CONNECTION_LOST);
byteswritten += size;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 406 is incorrect. It should be:

byteswritten += written;

Chuck

@earlephilhower earlephilhower added merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Oct 1, 2019
@earlephilhower
Copy link
Collaborator

Thanks for your PR, but the core and libraries have changed enough that this PR now has a merge conflict.

Could you merge it manually with the latest core, so we can consider it for future releases?

@dirkx
Copy link
Author

dirkx commented Oct 1, 2019 via email

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 1, 2019

I'm sorry I broke it while trying to merge by hand. @dirkx I'm going to PR on your fork to fix my mistake.
edit: fixed

@dirkx
Copy link
Author

dirkx commented Oct 2, 2019 via email

earlephilhower added a commit to earlephilhower/Arduino that referenced this pull request Nov 18, 2019
This is all @dirkx , whose PR unfortunately got borked when we were
trying to update it to the new format.  As @dirkx said:

When sending POST responses of well over a K - _write() may not sent it
all. Make sure we do -- but cap the individual writes - as somehow large
2-3k blobs seem to cause instability (on my 12F units).

Supercedes esp8266#2528
devyte pushed a commit that referenced this pull request Nov 24, 2019
This is all @dirkx , whose PR unfortunately got borked when we were
trying to update it to the new format.  As @dirkx said:

When sending POST responses of well over a K - _write() may not sent it
all. Make sure we do -- but cap the individual writes - as somehow large
2-3k blobs seem to cause instability (on my 12F units).

Supercedes #2528
@earlephilhower
Copy link
Collaborator

Superseded by #6800 which is now merged. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants