[2.7] bpo-36337: Use ssize_t and size_t for socket.send()/sendall()

[matrixise]

This PR is not yet completed (missing tests), I have "backported" a patch from 3.x.

https://bugs.python.org/issue36337

[vstinner]

I don't think that a test is needed: I cannot find a test on master.

Your PR is wrong on Windows. Look at the master branch:

#ifdef MS_WINDOWS
    if (ctx->len > INT_MAX)
        ctx->len = INT_MAX;
    ctx->result = send(s->sock_fd, ctx->buf, (int)ctx->len, ctx->flags);
#else
    ctx->result = send(s->sock_fd, ctx->buf, ctx->len, ctx->flags);
#endif

On Windows, the type of send() length argument is int, not size_t.

[vstinner]

return PyInt_FromLong((long)n); below is wrong

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[matrixise]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

[vstinner]

That's wrong: sendsegmented() len type is int, not Py_ssize_t. Don't forget the VMS! :-)

[vstinner]

I propose:

Fix buffer overflow in :meth:~socket.socket.send and :meth:~socket.socket.sendall methods of :func:socket.socket for data larger than 2 GiB.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[matrixise]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

[vstinner]

(int)len is unsafe if len is larger than INT_MAX. You should modify sendsegmented() to accept Py_ssize_t rather than int.

[matrixise]

👍 good catch and I was thinking about this problem.

[vstinner]

LGTM.

The subtle side effect of this change is that send() and sendall() now always return a Python 'long' object, whereas they always returned a Python 'int' object previously. I think that it's acceptable, but it might be risky to push so change late in 2.7 lifetime (EOL scheduled at the beginning of next year).

On the other side, currently, calling sendall() with more than 2 GiB data loose data. But well, who does that with Python 2.7?

I would prefer to have the review of another core dev.

@benjaminp @serhiy-storchaka @pablogsal: Would you mind to review this change? Do you think that it's ok to change send/sendall return type (int => long, supposed to be compatible)?

[matrixise]

I have made the requested changes; please review again

@serhiy-storchaka @vstinner

[bedevere-bot]

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

[pablogsal]

Wait, is this cast safe? In C99 ssize_t can be an unsigned long, unless I am missing something. I think you need:

Py_SAFE_DOWNCAST(remaining, Py_ssize_t, unsigned int)

[vstinner]

SEGMENT_SIZE is 32 KiB: remaining is only casted for values between [0; 32 KiB]:

#define SEGMENT_SIZE (32 * 1024 -1)

[pablogsal]

LGTM