Please, bump the major version number when you break the API

[bassosimone]

Thank you for your effort in making and crafting uTLS!

Because uTLS has a version number >= 1, the expectation in the Go world is that you should bump the major version number when breaking the API.

I would like to recommend you follow this best practice, because not following it causes confusion (if not issues) downstream.

Case in point:

• ooniprobe depends on Psiphon and obfs4;
• we want to use lyrebird instead of obfs4, so that our tests use the same obfs4 codebase used by tor;
• we cannot switch to lyrebird instead of obfs4 because the version of uTLS used by Psiphon (v1.3.3) is not API compatible with the version used by lyrebird (v1.5.3): chore: replace obfs4 with lyrebird ooni/probe-cli#1348.

If you check the build failure at ooni/probe-cli#1348, this is the build error after replacing obfs4 with lyrebird:

$ go build -v ./...
github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol
/home/sbs/go/pkg/mod/github.com/!psiphon-!labs/[email protected]/psiphon/common/protocol/customTLSProfiles.go:180:16: undefined: utls.UtlsExtendedMasterSecretExtension

Now, reading the diff between v1.3.3 and v1.5.3, I see you renamed UtlsExtendedMasterSecretExtension to ExtendedMasterSecretExtension, which is what breaks the ooniprobe build (and it seems there are also other renames).

In such a case, it would have been more consistent with the expectations for a Go project to tag v2. At least, this change would have signalled to anyone using uTLS that there was a API breakage. (It does not mean that ooniprobe would have been able to build both lyrebird instead of obfs4 and Psiphon, but that's off topic with respect to this issue.)

Thank you!

[gaukas]

Thanks for bringing this up.

I apologize for any inconvenience this has caused, this should not happen and I am terribly sorry. I will keep this in mind for any upcoming changes and try not to introduce conflicting APIs in the same major version.

On a different note, I have to mention that uTLS, being a superset of crypto/tls, choose to respect all changes made to crypto/tls even if they could be breaking. It is my personal observation that Go std libraries are not being completely backward compatible across minor versions. In the most recent minor release (Go 1.21) quite a few existing APIs have been broken in crypto/tls. It feels weird to bump up uTLS major version in this case while we want to try to not introduce a uTLS2 without really major improvements. It's still something I haven't been able to figure out, so I'm open for suggestions for the overall versioning policies for uTLS.

[gaukas]

And if it helps, I can definitely add the removed symbols back if you could provide a list of the missing symbols.

Again the intention for the symbol renaming is totally for the documentation accuracy, since Go 1.21 officially implements EMS, we are instead using the crypto/tls's implementation and therefore noting in the name that this extension is no longer uTLS-exclusive.

[gaukas]

In the latest tagged version I added u_alias.go to "undo breaking" some of the APIs. For now I'm only linking UtlsExtendedMasterSecretExtension to ExtendedMasterSecretExtension but we can add more if needed.

However some of the upstream introduced breaking update (e.g., TLS13OnlyState in u_public.go is no longer compatible with older versions since v1.5.0 due to clientHandshakeStateTLS13's implementation has been changed, or renamings due to crypto/tls started to use one of the existing names in uTLS), we still need a good approach to a fix.

Let me know if you have other suggestions/comments, or would like me to "unbreak" some other changes by adding/linking some names back, I'd be happy to help.

I will close this issue for now, but we can reopen or start a new one when needed. Thanks again and sorry for the inconvenience.

[bassosimone]

Thanks for bringing this up.

I apologize for any inconvenience this has caused, this should not happen and I am terribly sorry. I will keep this in mind for any upcoming changes and try not to introduce conflicting APIs in the same major version.

No worries! Rather, thank you for maintaining uTLS 🙌 !

On a different note, I have to mention that uTLS, being a superset of crypto/tls, choose to respect all changes made to crypto/tls even if they could be breaking. It is my personal observation that Go std libraries are not being completely backward compatible across minor versions. In the most recent minor release (Go 1.21) quite a few existing APIs have been broken in crypto/tls. It feels weird to bump up uTLS major version in this case while we want to try to not introduce a uTLS2 without really major improvements. It's still something I haven't been able to figure out, so I'm open for suggestions for the overall versioning policies for uTLS.

Great point! I have some direct, limited experience with this kind of issues because of our crypto/tls fork (whose set of patches on top of crypto/tls is very significantly smaller than the one in uTLS).

An alternative approach could perhaps have been to avoid tagging v1.0, given that v1.0 comes with its own sets of constraints in terms of API stability and import path changes.

But, given that v1.0 has already been released, this approach does not seem possible anymore.

And if it helps, I can definitely add the removed symbols back if you could provide a list of the missing symbols.

Again the intention for the symbol renaming is totally for the documentation accuracy, since Go 1.21 officially implements EMS, we are instead using the crypto/tls's implementation and therefore noting in the name that this extension is no longer uTLS-exclusive.

Thank you for explaining! It definitely makes sense to perform this kind of renaming in light of the upstream changes.

In the latest tagged version I added u_alias.go to "undo breaking" some of the APIs. For now I'm only linking UtlsExtendedMasterSecretExtension to ExtendedMasterSecretExtension but we can add more if needed.

Thank you for this! 🙌

However some of the upstream introduced breaking update (e.g., TLS13OnlyState in u_public.go is no longer compatible with older versions since v1.5.0 due to clientHandshakeStateTLS13's implementation has been changed, or renamings due to crypto/tls started to use one of the existing names in uTLS), we still need a good approach to a fix.

Thank you for further explaining other changes!

Let me know if you have other suggestions/comments, or would like me to "unbreak" some other changes by adding/linking some names back, I'd be happy to help.

For now, I have managed to find a compromise solution that has fixed the build of ooniprobe, where we're pinning to a version supported by psiphon and snowflake at the same time.

Regarding suggestions, the only one that comes to my mind is to investigate how the Go project manages to ensure they're not breaking their public API. My (limited) understanding is that they have a directory (https://github.com/golang/go/tree/master/api) in which they put textual files representing APIs, and run CI checks to make sure that new commits are not breaking the existing API definitions. Perhaps, this approach could be adapted to uTLS, and would allow you to know when a symbol that was previously existing has disappeared due to merging with upstream. The source code that implements these checks seems to be in ./src/cmd/api.

[bassosimone]

@gaukas I have been thinking a bit more about this whole issue, especially after analyzing the situation of cross compatibility between Psiphon and Snowflake, which are two dependencies of OONI. They both depend on uTLS and the versions they depend on are incompatible, so we cannot upgrade Snowflake beyond v2.6.2.

Anyway, while thinking it occurred me that, if you bumped the major version number every time you chase a new version of Go and/or introduce new functionality (which maybe is a bit overkill but at least it's a simple rule of thumb), the problem we're having of mutually incompatible versions would disappear. In fact, by bumping the major version number, the import path changes, and we would therefore be able to build both the latest Snowflake and the latest Psiphon at the same time.

What do you think? Does that make sense to you?