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

Jump to bottom

gskit_connect_step1: fixed bogus setsockopt calls #8793

Closed
wants to merge 1 commit into from
Closed

gskit_connect_step1: fixed bogus setsockopt calls #8793

wants to merge 1 commit into from

Conversation

piru
Copy link

@piru piru commented May 4, 2022

setsockopt takes a reference to value, not value. With the current code this just leads to -1 return value with errno EFAULT.

gskit_connect_step1: fixed bogus setsockopt calls
fe3e030 
setsockopt takes a reference to value, not value. With the current
code this just leads to -1 return value with errno EFAULT.
bagder
bagder approved these changes May 4, 2022
View reviewed changes
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

It is curious that it has been this wrong all since cb4e2be when these lines were introduced...

@bagder bagder added the TLS label May 4, 2022
@bagder bagder closed this in 1a78051 May 4, 2022
@bagder
Copy link
Member

bagder commented May 4, 2022

Thanks!

@monnerat
Copy link
Contributor

monnerat commented May 4, 2022 •
edited

It is curious that it has been this wrong all since cb4e2be when these lines were introduced...

2 possible explanations:

  1. The system default values are satisfactory
  2. No one ever securely transmitted large data via an https proxy from OS/400 libcurl (I don't remember if I tested it) ! I remember having tested it while developing with small data only that would have fulfill explanation 1.

Note that the sytem defaults are 65535, which is larger than the standard CURL_MAX_WRITE_SIZE (16384). However this can be overridden for a given job: see https://www.ibm.com/docs/es/i/7.1?topic=ssw_ibm_i_71/cl/chgtcpa.htm#CHGTCPA.TCPRCVBUF

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
TLS
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@piru @bagder @monnerat