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

Upgrade to go 1.23 #466

Merged
merged 6 commits into from
Jan 22, 2025
Merged

Upgrade to go 1.23 #466

merged 6 commits into from
Jan 22, 2025

Conversation

thecsw
Copy link
Contributor

@thecsw thecsw commented Sep 3, 2024

Following upon #461, updating the go version to latest, the 1.23 release gives us many new improvements in both the compiler land and tooling land to utilize:

  • Profile Guided Optimizations (PGOs) straight with the compiler allows for 2-7% perf improvements. GA in 1.21.
  • Built-in min, max, so we don't have to constantly implement our own, see: utility.go:129, modules/mssql/connection.go:749, modules/oracle/types_test.go:455, lib/ssh/channel.go:134
  • Structured logging in the standard library!
  • Slices and maps helpers in the standard library.
  • ...and many other improvements all around that we would get for free with all the new tooling to improve zgrab2 further.

How to Test

CI/CD involved should handle it. As Go team noted in all release notes since 1.20 (current version in this project) to now,

As always, the release maintains the Go 1 promise of compatibility. We expect almost all Go programs to continue to compile and run as before.

Compiling and running the program yields no issues.

Notes & Caveats

Only one "breaking" compile-time issue found in modules/amqp091/scanner.go:250, where ChannelMax was expecting an int but got an uint16. Casting here is safe as this is a type promotion with no requirements for this variable to be exactly 16 bits.

Issue Tracking

Add a link to the relevant GitHub issue(s) if the pull request resolves it.

@phillip-stephens
Copy link
Contributor

Hey @thecsw, thanks for this!

Off-hand, though, I'm hesitant to want to bump this to Go 1.23 for backwards-compatibility. We have users using older OS's and don't want to preclude them from being able to compile from source. All the compiler-driven enhancements you mention can still be obtained today by using the latest Go compiler on the existing source code. I don't think the other built-ins standard library enhancements can justify decreasing backwards compatibility, IMO.

@thecsw
Copy link
Contributor Author

thecsw commented Sep 3, 2024

Sure thing and totally valid! With this bump, it would require, at least on major systems, to run at least Big Sur for Mac and windows 10 (some new ports for bsd/etc. introduced)—makes sense for compat

We could possibly leave this open/archived for whenever time comes to upgrade to go1.23, at least

@phillip-stephens
Copy link
Contributor

After the discussion on zmap/zdns#495, I think we can go ahead with this!

@thecsw
Copy link
Contributor Author

thecsw commented Dec 31, 2024

@phillip-stephens wonderful—I noticed there are some new test failures since the last good head—I'll review those and make changes to get this through!

@thecsw
Copy link
Contributor Author

thecsw commented Dec 31, 2024

Okay, rebased onto the upstream's master and should be good to go

@thecsw
Copy link
Contributor Author

thecsw commented Jan 19, 2025

The failures seem to be coming from the python integration tests,

One or more schema validations failed: schema failure@ftp/authtls.json, schema failure@http/https.json, schema failure@ipp/cups-tls.json, schema failure@mssql/2017-linux.json, schema failure@mysql/5.7.json, schema failure@mysql/8.0.json, schema failure@postgres/10.1-ssl.json, schema failure@postgres/9.3-ssl.json, schema failure@postgres/9.4-ssl.json, schema failure@postgres/9.5-ssl.json, schema failure@postgres/9.6-ssl.json

Not super familiar with them or how these are checked, may be transient or coming from upstream

@Seanstoppable
Copy link
Contributor

Looks like all the failures are in the exact same place:
ERROR:root:data.ftp.result.tls.handshake_log.server_hello.compression_method: class mismatch for compression_method: expected (<class 'future.types.newint.newint'>,), {u'hex': u'0x00', u'name': u'NULL', u'value': 0} has class dict
This does look like a zcrypto change, where compression_method was changed from a uint to a struct here: zmap/zcrypto@c1c1128
Though that commit is 2 years old!

@phillip-stephens phillip-stephens merged commit ee17928 into zmap:master Jan 22, 2025
4 checks passed
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.

3 participants