Skip to content

Feature implementation from commits 17207fc..5f3ae64#4

Open
codeOwlAI wants to merge 15 commits intofeature-base-branch-4from
feature-head-branch-4
Open

Feature implementation from commits 17207fc..5f3ae64#4
codeOwlAI wants to merge 15 commits intofeature-base-branch-4from
feature-head-branch-4

Conversation

@codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 30, 2025

PR Summary

Socket Options and JSON Serialization Enhancements

Overview

This PR enhances socket options configuration with interface binding support and improves JSON serialization for DNS-related structs. It includes breaking changes to protobuf field numbering.

Change Types

Type Description
Feature Added socket binding to specific network interfaces
Enhancement Improved JSON serialization for DNS-related structs
Refactor Replaced custom network functions with standard net.ListenConfig
Breaking Renumbered protobuf fields in CustomSockopt message

Affected Modules

Module / File Change Description
conf/dns.go Added JSON marshaling support for HostsWrapper struct
conf/fakedns.go Added MarshalJSON method to FakeDNSConfig for handling mutually exclusive fields
internet/config.pb.go Added new 'system' field and renumbered protobuf fields (breaking change)
internet/sockopt_darwin.go Added interface binding and custom socket options for Darwin
internet/sockopt_windows.go Added IPv6 and custom socket options for Windows
internet/system_dialer.go Refactored to use standard net.ListenConfig

Breaking Changes

  • Renumbered all existing fields in the CustomSockopt protobuf definition, which breaks wire compatibility with existing clients
  • Removed getInterfaceIndexByName function from sockopt_darwin.go
  • Removed IP_MULTICAST_IF and IPV6_MULTICAST_IF constants from sockopt_windows.go

Notes for Reviewers

  • Verify that the new socket binding functionality works correctly across platforms
  • Ensure JSON serialization/deserialization is symmetric for DNS-related structs
  • Check for potential compatibility issues with existing clients due to protobuf field renumbering

RPRX and others added 15 commits March 31, 2025 10:09
Completes XTLS#1677

---------

Co-authored-by: RPRX <63339210+RPRX@users.noreply.github.com>
Bumps [github.com/miekg/dns](https://github.com/miekg/dns) from 1.1.64 to 1.1.65.
- [Changelog](https://github.com/miekg/dns/blob/master/Makefile.release)
- [Commits](miekg/dns@v1.1.64...v1.1.65)

---
updated-dependencies:
- dependency-name: github.com/miekg/dns
  dependency-version: 1.1.65
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.71.0 to 1.71.1.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.71.0...v1.71.1)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.31.0 to 0.32.0.
- [Commits](golang/sys@v0.31.0...v0.32.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-version: 0.32.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.12.0 to 0.13.0.
- [Commits](golang/sync@v0.12.0...v0.13.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sync
  dependency-version: 0.13.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.36.0 to 0.37.0.
- [Commits](golang/crypto@v0.36.0...v0.37.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-version: 0.37.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…shalJSON (XTLS#4585)

* conf: implement MarshalJSON for FakeDNSConfig

* conf: Rewrite MarshalJSON for PortList
decouple PortRange from PortList.

* conf: implement MarshalJSON for HostAddress

* conf: Add MarshalJSON comments and use pointers.
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.38.0 to 0.39.0.
- [Commits](golang/net@v0.38.0...v0.39.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-version: 0.39.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/cloudflare/circl](https://github.com/cloudflare/circl) from 1.6.0 to 1.6.1.
- [Release notes](https://github.com/cloudflare/circl/releases)
- [Commits](cloudflare/circl@v1.6.0...v1.6.1)

---
updated-dependencies:
- dependency-name: github.com/cloudflare/circl
  dependency-version: 1.6.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* WireGuard: Fix tunnel not closed

* Dialer: Apply controllers in lc.Control
* Sockopt: Add custom sockopt on Windows & Darwin

* fix windows udp by the way

* use resolved addr

XTLS#4504 (comment)
0x6f, 0x33,
0x70, 0x6f, 0x72, 0x74, 0x4c, 0x61, 0x79, 0x65, 0x72, 0x50, 0x72, 0x6f, 0x78, 0x79, 0x22, 0x93,
0x01, 0x0a, 0x0d, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x53, 0x6f, 0x63, 0x6b, 0x6f, 0x70, 0x74,
0x12, 0x16, 0x0a, 0x06, 0x73, 0x79, 0x73, 0x74, 0x65, 0x6d, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09,
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Breaking Protobuf Field Numbering Change.

The change renumbers all existing fields in the CustomSockopt message, which breaks wire compatibility with any existing serialized data or clients.

Proposed Code:

12, 0x16, 0x0a, 0x06, 0x73, 0x79, 0x73, 0x74, 0x65, 0x6d, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x73, 0x79, 0x73, 0x74, 0x65, 0x6d

if len(custom.Opt) == 0 {
return errors.New("No opt!")
} else {
opt, _ = strconv.Atoi(custom.Opt)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error return from strconv.Atoi().

The code ignores the error return from strconv.Atoi(), which could lead to using an invalid socket option value if the conversion fails.

Current Code (Diff):

- 				opt, _ = strconv.Atoi(custom.Opt)
+ 				var parseErr error
+ 				opt, parseErr = strconv.Atoi(custom.Opt)
+ 				if parseErr != nil {
+ 					return errors.New("failed to parse socket option").Base(parseErr)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
opt, _ = strconv.Atoi(custom.Opt)
var parseErr error
opt, parseErr = strconv.Atoi(custom.Opt)
if parseErr != nil {
return errors.New("failed to parse socket option").Base(parseErr)
}

opt, _ = strconv.Atoi(custom.Opt)
}
if custom.Level != "" {
level, _ = strconv.Atoi(custom.Level)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error return from strconv.Atoi().

The code ignores the error return from strconv.Atoi() for the socket level, which could lead to using an invalid socket level if the conversion fails.

Current Code (Diff):

- 				level, _ = strconv.Atoi(custom.Level)
+ 				var parseErr error
+ 				level, parseErr = strconv.Atoi(custom.Level)
+ 				if parseErr != nil {
+ 					return errors.New("failed to parse socket level").Base(parseErr)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
level, _ = strconv.Atoi(custom.Level)
var parseErr error
level, parseErr = strconv.Atoi(custom.Level)
if parseErr != nil {
return errors.New("failed to parse socket level").Base(parseErr)
}

level, _ = strconv.Atoi(custom.Level)
}
if custom.Type == "int" {
value, _ := strconv.Atoi(custom.Value)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error return from strconv.Atoi().

The code ignores the error return from strconv.Atoi() for the socket option value, which could lead to using an invalid value if the conversion fails.

Current Code (Diff):

- 				value, _ := strconv.Atoi(custom.Value)
+ 				value, err := strconv.Atoi(custom.Value)
+ 				if err != nil {
+ 					return errors.New("failed to parse socket option value").Base(err)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
value, _ := strconv.Atoi(custom.Value)
value, err := strconv.Atoi(custom.Value)
if err != nil {
return errors.New("failed to parse socket option value").Base(err)
}

if !strings.HasPrefix(network, custom.Network) {
continue
}
var level = 0x6 // default TCP
Copy link

Choose a reason for hiding this comment

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

🔒 Security Issue

Hardcoded socket level constant.

Using a hardcoded hex value (0x6) for the socket level instead of a proper symbolic constant creates a maintenance risk and potential security issue.

Current Code (Diff):

- 			var level = 0x6 // default TCP
+ 			var level = unix.IPPROTO_TCP // default TCP
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
var level = 0x6 // default TCP
var level = unix.IPPROTO_TCP // default TCP

if len(custom.Opt) == 0 {
return errors.New("No opt!")
} else {
opt, _ = strconv.Atoi(custom.Opt)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error from strconv.Atoi() for socket option.

The code discards the error from strconv.Atoi(), which could cause unpredictable behavior if the conversion fails and silently uses 0 as the option value.

Current Code (Diff):

-				opt, _ = strconv.Atoi(custom.Opt)
+				var err error
+				opt, err = strconv.Atoi(custom.Opt)
+				if err != nil {
+					return errors.New("failed to parse socket option").Base(err)
+				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
opt, _ = strconv.Atoi(custom.Opt)
var err error
opt, err = strconv.Atoi(custom.Opt)
if err != nil {
return errors.New("failed to parse socket option").Base(err)
}

opt, _ = strconv.Atoi(custom.Opt)
}
if custom.Level != "" {
level, _ = strconv.Atoi(custom.Level)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error from strconv.Atoi() for level value.

The code discards the error from strconv.Atoi() for the level value, which could lead to using incorrect socket option levels if conversion fails.

Current Code (Diff):

-				level, _ = strconv.Atoi(custom.Level)
+				var err error
+				level, err = strconv.Atoi(custom.Level)
+				if err != nil {
+					return errors.New("failed to parse socket level").Base(err)
+				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
level, _ = strconv.Atoi(custom.Level)
var err error
level, err = strconv.Atoi(custom.Level)
if err != nil {
return errors.New("failed to parse socket level").Base(err)
}

level, _ = strconv.Atoi(custom.Level)
}
if custom.Type == "int" {
value, _ := strconv.Atoi(custom.Value)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignoring error from strconv.Atoi() for option value.

The code discards the error from strconv.Atoi() when parsing the option value, potentially causing undefined socket behavior with invalid inputs.

Current Code (Diff):

-				value, _ := strconv.Atoi(custom.Value)
+				value, err := strconv.Atoi(custom.Value)
+				if err != nil {
+					return errors.New("failed to parse socket option value").Base(err)
+				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
value, _ := strconv.Atoi(custom.Value)
value, err := strconv.Atoi(custom.Value)
if err != nil {
return errors.New("failed to parse socket option value").Base(err)
}

var level = 0x6 // default TCP
var opt int
if len(custom.Opt) == 0 {
return errors.New("No opt!")
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Uninformative error message.

The error message "No opt!" is not descriptive enough for debugging and doesn't match the style of other error messages in the codebase.

Current Code (Diff):

-				return errors.New("No opt!")
+				return errors.New("missing required socket option value")
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return errors.New("No opt!")
return errors.New("missing required socket option value")

if len(custom.Opt) == 0 {
return errors.New("No opt!")
} else {
opt, _ = strconv.Atoi(custom.Opt)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignored Conversion Error in Socket Option.

The code ignores potential Atoi conversion errors when parsing custom socket options, which could cause invalid socket configurations if non-numeric values are provided.

Current Code (Diff):

- 				opt, _ = strconv.Atoi(custom.Opt)
+ 				opt, err := strconv.Atoi(custom.Opt)
+ 				if err != nil {
+ 					return errors.New("Invalid socket option value").Base(err)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
opt, _ = strconv.Atoi(custom.Opt)
opt, err := strconv.Atoi(custom.Opt)
if err != nil {
return errors.New("Invalid socket option value").Base(err)
}

opt, _ = strconv.Atoi(custom.Opt)
}
if custom.Level != "" {
level, _ = strconv.Atoi(custom.Level)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignored Conversion Error in Socket Level.

The code ignores potential Atoi conversion errors when parsing the socket level, which could apply incorrect socket configurations if non-numeric values are provided.

Current Code (Diff):

- 				level, _ = strconv.Atoi(custom.Level)
+ 				level, err := strconv.Atoi(custom.Level)
+ 				if err != nil {
+ 					return errors.New("Invalid socket level value").Base(err)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
level, _ = strconv.Atoi(custom.Level)
level, err := strconv.Atoi(custom.Level)
if err != nil {
return errors.New("Invalid socket level value").Base(err)
}

level, _ = strconv.Atoi(custom.Level)
}
if custom.Type == "int" {
value, _ := strconv.Atoi(custom.Value)
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Ignored Conversion Error in Socket Value.

The code ignores potential Atoi conversion errors when parsing the custom socket value, which could lead to incorrect socket options being applied.

Current Code (Diff):

- 				value, _ := strconv.Atoi(custom.Value)
+ 				value, err := strconv.Atoi(custom.Value)
+ 				if err != nil {
+ 					return errors.New("Invalid socket option value").Base(err)
+ 				}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
value, _ := strconv.Atoi(custom.Value)
value, err := strconv.Atoi(custom.Value)
if err != nil {
return errors.New("Invalid socket option value").Base(err)
}

if !strings.HasPrefix(network, custom.Network) {
continue
}
var level = 0x6 // default TCP
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Hardcoded Protocol Assumption.

The code hardcodes level 0x6 (IPPROTO_TCP) without verifying the socket type, which could cause incorrect socket configurations for non-TCP protocols.

Current Code (Diff):

- 			var level = 0x6 // default TCP
+ 			var level int
+ 			if strings.HasPrefix(network, "tcp") {
+ 				level = 0x6 // IPPROTO_TCP
+ 			} else if strings.HasPrefix(network, "udp") {
+ 				level = 0x11 // IPPROTO_UDP
+ 			} else {
+ 				level = 0x0 // SOL_SOCKET for general socket options
+ 			}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
var level = 0x6 // default TCP
var level int
if strings.HasPrefix(network, "tcp") {
level = 0x6 // IPPROTO_TCP
} else if strings.HasPrefix(network, "udp") {
level = 0x11 // IPPROTO_UDP
} else {
level = 0x0 // SOL_SOCKET for general socket options
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants