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.

Sign up for GitHub

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

Jump to bottom

Dont ignore json marshal errors #2435

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Dont ignore json marshal errors #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion agent/api/task/task.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,10 @@ func (task *Task) addNetworkResourceProvisioningDependency(cfg *config.Config) e
Image: fmt.Sprintf("%s:%s", cfg.PauseContainerImageName, cfg.PauseContainerTag),
}

bytes, _ := json.Marshal(pauseConfig)
bytes, err := json.Marshal(pauseConfig)
if err != nil {
return errors.Errorf("Error json marshaling pause config: %s", err)
}
serializedConfig := string(bytes)
pauseContainer.DockerConfig = apicontainer.DockerConfig{
Config: &serializedConfig,
Expand Down
5 changes: 4 additions & 1 deletion agent/handlers/introspection_server_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ func introspectionServerSetup(containerInstanceArn *string, taskEngine handlersu
paths := []string{v1.AgentMetadataPath, v1.TaskContainerMetadataPath, v1.LicensePath}
availableCommands := &rootResponse{paths}
// Autogenerated list of the above serverFunctions paths
availableCommandResponse, _ := json.Marshal(&availableCommands)
availableCommandResponse, err := json.Marshal(&availableCommands)
if err != nil {
seelog.Errorf("Error marshaling JSON in introspection server setup: %s", err)
}

defaultHandler := func(w http.ResponseWriter, r *http.Request) {
w.Write(availableCommandResponse)
Expand Down
13 changes: 13 additions & 0 deletions agent/handlers/utils/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package utils

import (
"fmt"
"net/http"

"github.com/aws/amazon-ecs-agent/agent/logger/audit"
Expand Down Expand Up @@ -81,6 +82,18 @@ func WriteJSONToResponse(w http.ResponseWriter, httpStatusCode int, responseJSON
}
}

// WriteResponseIfMarshalError checks the 'err' response of the json.Marshal function.
// if this function returns an error, then it has already written a response to the
// http writer, and the calling function should return.
func WriteResponseIfMarshalError(w http.ResponseWriter, err error) error {
if err != nil {
WriteJSONToResponse(w, http.StatusInternalServerError, []byte(`{}`), RequestTypeAgentMetadata)
seelog.Errorf("Error marshaling json: %s", err)
return fmt.Errorf("json marshal error")
}
return nil
}

// ValueFromRequest returns the value of a field in the http request. The boolean value is
// set to true if the field exists in the query.
func ValueFromRequest(r *http.Request, field string) (string, bool) {
Expand Down
5 changes: 4 additions & 1 deletion agent/handlers/v1/agent_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ func AgentMetadataHandler(containerInstanceArn *string, cfg *config.Config) func
ContainerInstanceArn: containerInstanceArn,
Version: agentversion.String(),
}
responseJSON, _ := json.Marshal(resp)
responseJSON, err := json.Marshal(resp)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeAgentMetadata)
}
}
5 changes: 4 additions & 1 deletion agent/handlers/v1/credentials_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ func CredentialsHandler(credentialsManager credentials.Manager, auditLogger audi
func CredentialsHandlerImpl(w http.ResponseWriter, r *http.Request, auditLogger audit.AuditLogger, credentialsManager credentials.Manager, credentialsID string, errPrefix string) {
responseJSON, arn, roleType, errorMessage, err := processCredentialsRequest(credentialsManager, r, credentialsID, errPrefix)
if err != nil {
errResponseJSON, _ := json.Marshal(errorMessage)
errResponseJSON, err := json.Marshal(errorMessage)
if e := handlersutils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
writeCredentialsRequestResponse(w, r, errorMessage.HTTPErrorCode, audit.GetCredentialsEventType(roleType), arn, auditLogger, errResponseJSON)
return
}
Expand Down
18 changes: 15 additions & 3 deletions agent/handlers/v1/task_container_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,21 @@ const (

// createTaskResponse creates JSON response and sets the http status code for the task queried.
func createTaskResponse(task *apitask.Task, found bool, resourceID string, state dockerstate.TaskEngineState) ([]byte, int) {
var err error
var responseJSON []byte
status := http.StatusOK
if found {
containerMap, _ := state.ContainerMapByArn(task.Arn)
responseJSON, _ = json.Marshal(NewTaskResponse(task, containerMap))
responseJSON, err = json.Marshal(NewTaskResponse(task, containerMap))
if err != nil {
return []byte("{}"), http.StatusInternalServerError
}
} else {
seelog.Warn("Could not find requested resource: " + resourceID)
responseJSON, _ = json.Marshal(&TaskResponse{})
responseJSON, err = json.Marshal(&TaskResponse{})
if err != nil {
return []byte("{}"), http.StatusInternalServerError
}
status = http.StatusNotFound
}
return responseJSON, status
Expand All @@ -51,6 +58,7 @@ func createTaskResponse(task *apitask.Task, found bool, resourceID string, state
// 'taskarn' are specified in the request.
func TaskContainerMetadataHandler(taskEngine utils.DockerStateResolver) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
var err error
var responseJSON []byte
dockerTaskEngineState := taskEngine.State()
dockerID, dockerIDExists := utils.ValueFromRequest(r, dockerIDQueryField)
Expand Down Expand Up @@ -92,7 +100,11 @@ func TaskContainerMetadataHandler(taskEngine utils.DockerStateResolver) func(htt
w.WriteHeader(status)
} else {
// List all tasks.
responseJSON, _ = json.Marshal(NewTasksResponse(dockerTaskEngineState))
responseJSON, err = json.Marshal(NewTasksResponse(dockerTaskEngineState))
if err != nil {
responseJSON = []byte("")
w.WriteHeader(http.StatusInternalServerError)
}
}
w.Write(responseJSON)
}
Expand Down
25 changes: 20 additions & 5 deletions agent/handlers/v2/task_container_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ func TaskContainerMetadataHandler(state dockerstate.TaskEngineState, ecsClient a
return func(w http.ResponseWriter, r *http.Request) {
taskARN, err := getTaskARNByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("Unable to get task arn from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeTaskMetadata)
return
}
Expand All @@ -70,12 +73,18 @@ func TaskContainerMetadataHandler(state dockerstate.TaskEngineState, ecsClient a
func WriteContainerMetadataResponse(w http.ResponseWriter, containerID string, state dockerstate.TaskEngineState) {
containerResponse, err := NewContainerResponse(containerID, state)
if err != nil {
errResponseJSON, _ := json.Marshal("Unable to generate metadata for container '" + containerID + "'")
errResponseJSON, err := json.Marshal("Unable to generate metadata for container '" + containerID + "'")
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}

responseJSON, _ := json.Marshal(containerResponse)
responseJSON, err := json.Marshal(containerResponse)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeContainerMetadata)
}

Expand All @@ -84,11 +93,17 @@ func WriteTaskMetadataResponse(w http.ResponseWriter, taskARN string, cluster st
// Generate a response for the task
taskResponse, err := NewTaskResponse(taskARN, state, ecsClient, cluster, az, containerInstanceArn, propagateTags)
if err != nil {
errResponseJSON, _ := json.Marshal("Unable to generate metadata for task: '" + taskARN + "'")
errResponseJSON, err := json.Marshal("Unable to generate metadata for task: '" + taskARN + "'")
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskMetadata)
return
}

responseJSON, _ := json.Marshal(taskResponse)
responseJSON, err := json.Marshal(taskResponse)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeTaskMetadata)
}
25 changes: 20 additions & 5 deletions agent/handlers/v2/task_container_stats_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ func TaskContainerStatsHandler(state dockerstate.TaskEngineState, statsEngine st
return func(w http.ResponseWriter, r *http.Request) {
taskARN, err := getTaskARNByRequest(r, state)
if err != nil {
errResponseJSON, _ := json.Marshal(
errResponseJSON, err := json.Marshal(
fmt.Sprintf("Unable to get task arn from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskStats)
return
}
Expand All @@ -69,12 +72,18 @@ func WriteTaskStatsResponse(w http.ResponseWriter,
taskStatsResponse, err := NewTaskStatsResponse(taskARN, state, statsEngine)
if err != nil {
seelog.Warnf("Unable to get task stats for task '%s': %v", taskARN, err)
errResponseJSON, _ := json.Marshal("Unable to get task stats for: " + taskARN)
errResponseJSON, err := json.Marshal("Unable to get task stats for: " + taskARN)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskStats)
return
}

responseJSON, _ := json.Marshal(taskStatsResponse)
responseJSON, err := json.Marshal(taskStatsResponse)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeTaskStats)
}

Expand All @@ -85,11 +94,17 @@ func WriteContainerStatsResponse(w http.ResponseWriter,
statsEngine stats.Engine) {
dockerStats, err := statsEngine.ContainerDockerStats(taskARN, containerID)
if err != nil {
errResponseJSON, _ := json.Marshal("Unable to get container stats for: " + containerID)
errResponseJSON, err := json.Marshal("Unable to get container stats for: " + containerID)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerStats)
return
}

responseJSON, _ := json.Marshal(dockerStats)
responseJSON, err := json.Marshal(dockerStats)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeContainerStats)
}
45 changes: 36 additions & 9 deletions agent/handlers/v3/container_association_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,33 @@ func ContainerAssociationsHandler(state dockerstate.TaskEngineState) func(http.R
return func(w http.ResponseWriter, r *http.Request) {
containerID, err := GetContainerIDByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: unable to get container id from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociations)
return
}

taskARN, err := GetTaskARNByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: unable to get task arn from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociations)
return
}

associationType, err := GetAssociationTypeByRequest(r)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociations)
return
}
Expand All @@ -82,24 +91,33 @@ func ContainerAssociationHandler(state dockerstate.TaskEngineState) func(http.Re
return func(w http.ResponseWriter, r *http.Request) {
taskARN, err := GetTaskARNByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: unable to get task arn from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociation)
return
}

associationType, err := GetAssociationTypeByRequest(r)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociation)
return
}

associationName, err := GetAssociationNameByRequest(r)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container associations handler: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerAssociation)
return
}
Expand All @@ -113,19 +131,28 @@ func ContainerAssociationHandler(state dockerstate.TaskEngineState) func(http.Re
func writeContainerAssociationsResponse(w http.ResponseWriter, containerID, taskARN, associationType string, state dockerstate.TaskEngineState) {
associationsResponse, err := NewAssociationsResponse(containerID, taskARN, associationType, state)
if err != nil {
errResponseJSON, _ := json.Marshal(fmt.Sprintf("Unable to write container associations response: %s", err.Error()))
errResponseJSON, err := json.Marshal(fmt.Sprintf("Unable to write container associations response: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerAssociations)
return
}

responseJSON, _ := json.Marshal(associationsResponse)
responseJSON, err := json.Marshal(associationsResponse)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeContainerAssociations)
}

func writeContainerAssociationResponse(w http.ResponseWriter, taskARN, associationType, associationName string, state dockerstate.TaskEngineState) {
associationResponse, err := NewAssociationResponse(taskARN, associationType, associationName, state)
if err != nil {
errResponseJSON, _ := json.Marshal(fmt.Sprintf("Unable to write container association response: %s", err.Error()))
errResponseJSON, err := json.Marshal(fmt.Sprintf("Unable to write container association response: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerAssociation)
return
}
Expand Down
15 changes: 12 additions & 3 deletions agent/handlers/v3/container_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,29 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
return func(w http.ResponseWriter, r *http.Request) {
containerID, err := GetContainerIDByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container metadata handler: unable to get container ID from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerMetadata)
return
}
containerResponse, err := GetContainerResponse(containerID, state)
if err != nil {
errResponseJSON, _ := json.Marshal(err.Error())
errResponseJSON, err := json.Marshal(err.Error())
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}
seelog.Infof("V3 container metadata handler: writing response for container '%s'", containerID)

responseJSON, _ := json.Marshal(containerResponse)
responseJSON, err := json.Marshal(containerResponse)
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeContainerMetadata)
}
}
Expand Down
10 changes: 8 additions & 2 deletions agent/handlers/v3/container_stats_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,22 @@ func ContainerStatsHandler(state dockerstate.TaskEngineState, statsEngine stats.
return func(w http.ResponseWriter, r *http.Request) {
taskARN, err := GetTaskARNByRequest(r, state)
if err != nil {
errResponseJSON, _ := json.Marshal(
errResponseJSON, err := json.Marshal(
fmt.Sprintf("V3 container stats handler: unable to get task arn from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskStats)
return
}

containerID, err := GetContainerIDByRequest(r, state)
if err != nil {
responseJSON, _ := json.Marshal(
responseJSON, err := json.Marshal(
fmt.Sprintf("V3 container stats handler: unable to get container ID from request: %s", err.Error()))
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerStats)
return
}
Expand Down
Loading