Stop using html/template in U2M OAuth callback to restore linker DCE (fixes #343)#389
Closed
vikrantpuppala wants to merge 2 commits into
Closed
Stop using html/template in U2M OAuth callback to restore linker DCE (fixes #343)#389vikrantpuppala wants to merge 2 commits into
vikrantpuppala wants to merge 2 commits into
Conversation
The README documented only PAT (token:) authentication and a handful of connection parameters, with no guidance on several options that the driver already supports and doc.go already lists. Add a consolidated 'Supported connection parameters' table (catalog, schema, maxRows, timeout, userAgentEntry, useCloudFetch, maxDownloadThreads, authType, accessToken, clientID, clientSecret) and worked DSN examples for: - OAuth machine-to-machine auth (clientID/clientSecret) - OAuth user-to-machine auth (authType=OauthU2M) - setting the user agent (userAgentEntry) Docs-only change; parameter names, defaults, and behavior verified against internal/config/config.go and its existing tests. Closes #196 Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Reachable use of html/template in auth/oauth/u2m disabled the Go linker's dead-code elimination for the entire binary of any application importing this driver, significantly increasing binary size (up to ~20% in multi-library binaries). Rewrite the mostly-static OAuth callback page as plain string building with explicit html.EscapeString for the dynamic fields, removing the html/template (and transitively text/template) dependency from the reachable graph. In a minimal binary importing the driver this drops text/template symbols from 615 to 0 and shrinks the binary by ~39%. Delete the now-unused embedded templates/simple.html and add tests covering rendering and HTML escaping. Closes #343 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Collaborator
Author
|
Superseded by #390, which is branched cleanly off main (this branch accidentally included an unrelated README commit from docs/196-dsn-parameters). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reachable use of the stdlib
html/templatein packageauth/oauth/u2mdisabled the Go linker's public dead-code elimination (DCE) for the entire binary of any application that importsdatabricks-sql-go.html/templatebuilds ontext/template, whose reflection-based field evaluation forces the linker to retain code it would otherwise strip. This bloats consumer binaries — up to ~20% (40 MB) in multi-library binaries likexo/usql, as reported in #343.This PR rewrites the mostly-static OAuth callback page as plain string building with explicit
html.EscapeStringon the dynamic fields (the same approachxo/tblfmtused for a similar issue), removinghtml/template/text/templatefrom the reachable graph. The now-unused embeddedtemplates/simple.htmlis deleted.The dynamic fields remain HTML-escaped, so the escaping behavior of the previous
html/templateimplementation is preserved.Impact
Measured with a minimal
main.gothat imports the driver (the repro from the issue):text/template/html/templatesymbols (go tool nm)The size delta is larger than the driver's own footprint because re-enabling DCE benefits the whole binary, including other libraries.
Test plan
auth/oauth/u2m/html_template_test.go:TestRenderHTML(table-driven): basic field rendering + escaping of<,>,&,"; the conditional action-link block; the conditional code block; and that each page is a complete, well-formed document.TestInfoHTML: success page title/heading/content, no code block.TestErrorHTML: error page title and that the error message is HTML-escaped (no raw<script>leaks through).go build ./...,go vet ./auth/oauth/u2m/, andgofmt -lare all clean.go test ./auth/oauth/u2m/passes.go tool nmon a consumer binary that the template symbols are gone (615 → 0) and the binary shrinks as shown above.Notes
html/templateortext/templateusages exist in the codebase — this was the sole occurrence of the pattern.Unreleasedsection.Closes #343
This pull request and its description were written by Isaac.