Skip to content

feat: Adds CORS Support#17

Open
bosvos wants to merge 2 commits intomasterfrom
asg-5216-cors
Open

feat: Adds CORS Support#17
bosvos wants to merge 2 commits intomasterfrom
asg-5216-cors

Conversation

@bosvos
Copy link
Contributor

@bosvos bosvos commented Mar 5, 2026

Refs: ASG-5216

@bosvos bosvos requested a review from LeonLeibbrandt March 5, 2026 07:36
Copy link
Contributor

@LeonLeibbrandt LeonLeibbrandt left a comment

Choose a reason for hiding this comment

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

LGTM, very cool.

test/README.md Outdated

## Usage
1. Run `cors_test_server.go` with `go run test/cors_test_server.go`.
2. Open `http://localhost:8080` in your browser.
Copy link
Contributor

Choose a reason for hiding this comment

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

go run test/servefile.go

Also when hitting this url for the first time it gives you a full listing of the repo. The correct url is http://localhost:8085/test/cors_test.html

New functionality for router to add headers for CORS sites
New config to specify allowed sites
Small test server and script for testing

ASG-5216
@FritzOnFire FritzOnFire changed the title Asg 5216 cors feat: Adds CORS Support Mar 11, 2026
Copy link
Contributor

@FritzOnFire FritzOnFire left a comment

Choose a reason for hiding this comment

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

A whole bunch of small changes, but it should be good to merge after them.

Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like fluff, but please move this file one level deeper in case we need to use this folder for other more involved testing setups

New Dir: test/cors.

But then we can probably rename the file to test.html, as it would be obvious what the file is used for.

test/README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Same fluff comment as on the other file

test/README.md Outdated

## Files

### cors_test_server.go
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### cors_test_server.go
### serverfile.go

test/README.md Outdated
## Files

### cors_test_server.go
A minimal Go HTTP server that serves the `cors_test.html` file on `http://localhost:8080`. This allows you to easily load the test page in your browser.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A minimal Go HTTP server that serves the `cors_test.html` file on `http://localhost:8080`. This allows you to easily load the test page in your browser.
A minimal Go HTTP server that serves the `test.html` file on `http://localhost:8080`. This allows you to easily load the test page in your browser.

test/README.md Outdated
### cors_test_server.go
A minimal Go HTTP server that serves the `cors_test.html` file on `http://localhost:8080`. This allows you to easily load the test page in your browser.

### cors_test.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### cors_test.html
### test.html

Comment on lines +460 to +474
// Set the Access-Control-Allow-Origin header, based on allow-list
s.errorLog.Infof("Forwarding from \"%v\"", req.Header.Get("Origin"))
_, ok := s.configHttp.Origins[req.Header.Get("Origin")]
if ok {
w.Header().Set("Access-Control-Allow-Origin", req.Header.Get("Origin"))
w.Header().Set("Access-Control-Allow-Credentials", "true")

// Handle preflight OPTIONS requests
if req.Method == "OPTIONS" {
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
w.WriteHeader(http.StatusOK)
return
}
}
Copy link
Contributor

@FritzOnFire FritzOnFire Mar 11, 2026

Choose a reason for hiding this comment

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

Just to minimize the impact on our standard config.

Suggested change
// Set the Access-Control-Allow-Origin header, based on allow-list
s.errorLog.Infof("Forwarding from \"%v\"", req.Header.Get("Origin"))
_, ok := s.configHttp.Origins[req.Header.Get("Origin")]
if ok {
w.Header().Set("Access-Control-Allow-Origin", req.Header.Get("Origin"))
w.Header().Set("Access-Control-Allow-Credentials", "true")
// Handle preflight OPTIONS requests
if req.Method == "OPTIONS" {
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
w.WriteHeader(http.StatusOK)
return
}
}
if len(s.configHttp.Origins) > 0 {
// Set the Access-Control-Allow-Origin header, based on allow-list
og := req.Header.Get("Origin")
s.errorLog.Infof("Forwarding from \"%v\"", og)
if _, ok := s.configHttp.Origins[og]; ok {
w.Header().Set("Access-Control-Allow-Origin", og)
w.Header().Set("Access-Control-Allow-Credentials", "true")
// Handle preflight OPTIONS requests
if req.Method == "OPTIONS" {
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
w.WriteHeader(http.StatusOK)
return
}
}
}

Fixup CORS file server
Fixup docs
Move cors tests and resources to /test/cors
Copy link
Contributor Author

@bosvos bosvos left a comment

Choose a reason for hiding this comment

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

I've made the other changes apart from the pointer one. @FritzOnFire

@bosvos bosvos requested a review from FritzOnFire March 11, 2026 16:42
Copy link
Contributor

@FritzOnFire FritzOnFire left a comment

Choose a reason for hiding this comment

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

Just that extra if statement in server.go but then this is good to merge

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