Skip to content

feat: Adds CORS Support#17

Open
bosvos wants to merge 2 commits into
masterfrom
asg-5216-cors
Open

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

Conversation

@bosvos

@bosvos bosvos commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Refs: ASG-5216

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

@LeonLeibbrandt LeonLeibbrandt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, very cool.

Comment thread 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
Copy Markdown
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

@FritzOnFire FritzOnFire left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread test/cors/test.html

Copy link
Copy Markdown
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.

Comment thread test/README.md Outdated

Copy link
Copy Markdown
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

Comment thread test/README.md Outdated

## Files

### cors_test_server.go

Copy link
Copy Markdown
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

Comment thread 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
Copy Markdown
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.

Comment thread 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
Copy Markdown
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 thread server/server.go
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
}
}

@FritzOnFire FritzOnFire Mar 11, 2026

Copy link
Copy Markdown
Contributor

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
}
}
}

Comment thread server/logic_test.go
Comment thread server/logic_test.go
Comment thread server/logic_test.go
Comment thread server/logic_test.go
Fixup CORS file server
Fixup docs
Move cors tests and resources to /test/cors

@bosvos bosvos left a comment

Copy link
Copy Markdown
Contributor Author

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

@FritzOnFire FritzOnFire left a comment

Copy link
Copy Markdown
Contributor

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