rob05c commented on a change in pull request #2269: Deliveryservice_Server API
conversion to Go
URL:
https://github.com/apache/incubator-trafficcontrol/pull/2269#discussion_r188111742
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
##########
@@ -97,130 +100,107 @@ func (dss *TODeliveryServiceServer) Validate(db
*sqlx.DB) []error {
return tovalidate.ToErrors(errs)
}
-//The TODeliveryServiceServer implementation of the Creator interface
-//all implementations of Creator should use transactions and return the proper
errorType
-//ParsePQUniqueConstraintError is used to determine if a profileparameter with
conflicting values exists
-//if so, it will return an errorType of DataConflict and the type should be
appended to the
-//generic error message returned
-//The insert sql returns the profile and lastUpdated values of the newly
inserted profileparameter and have
-//to be added to the struct
-func (dss *TODeliveryServiceServer) Create(db *sqlx.DB, user auth.CurrentUser)
(error, tc.ApiErrorType) {
- rollbackTransaction := true
- tx, err := db.Beginx()
- defer func() {
- if tx == nil || !rollbackTransaction {
- return
+// api/1.1/deliveryserviceserver$
+func ReadDSSHandler(db *sqlx.DB) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ //create error function with ResponseWriter and Request
+ handleErrs := tc.GetHandleErrorsFunc(w, r)
+
+ ctx := r.Context()
+
+ // Load the PathParams into the query parameters for pass
through
+ params, err := api.GetCombinedParams(r)
+ if err != nil {
+ log.Errorf("unable to get parameters from request: %s",
err)
+ handleErrs(http.StatusInternalServerError, err)
}
- err := tx.Rollback()
+
+ user, err := auth.GetCurrentUser(ctx)
if err != nil {
- log.Errorln(errors.New("rolling back transaction: " +
err.Error()))
+ log.Errorf("unable to retrieve current user from
context: %s", err)
+ handleErrs(http.StatusInternalServerError, err)
+ return
}
- }()
- if err != nil {
- log.Error.Printf("could not begin transaction: %v", err)
- return tc.DBError, tc.SystemError
- }
- resultRows, err := tx.NamedQuery(insertQuery(), dss)
- if err != nil {
- if pqErr, ok := err.(*pq.Error); ok {
- err, eType :=
dbhelpers.ParsePQUniqueConstraintError(pqErr)
- if eType == tc.DataConflictError {
- return errors.New("a parameter with " +
err.Error()), eType
- }
- return err, eType
+ results, errs, errType := GetRefType().readDSS(db, params,
*user)
+ if len(errs) > 0 {
+ tc.HandleErrorsWithType(errs, errType, handleErrs)
+ return
}
- log.Errorf("received non pq error: %++v from create execution",
err)
- return tc.DBError, tc.SystemError
- }
- defer resultRows.Close()
-
- var ds_id int
- var server_id int
- var lastUpdated tc.TimeNoMod
- rowsAffected := 0
- for resultRows.Next() {
- rowsAffected++
- if err := resultRows.Scan(&ds_id, &server_id, &lastUpdated);
err != nil {
- log.Error.Printf("could not scan dss from insert:
%s\n", err)
- return tc.DBError, tc.SystemError
+ respBts, err := json.Marshal(results)
+ if err != nil {
+ handleErrs(http.StatusInternalServerError, err)
+ return
}
- }
- if rowsAffected == 0 {
- err = errors.New("no deliveryServiceServer was inserted,
nothing to return")
- log.Errorln(err)
- return tc.DBError, tc.SystemError
- }
- if rowsAffected > 1 {
- err = errors.New("too many ids returned from parameter insert")
- log.Errorln(err)
- return tc.DBError, tc.SystemError
- }
- dss.SetKeys(map[string]interface{}{"deliveryservice": ds_id, "server":
server_id})
- dss.LastUpdated = &lastUpdated
- err = tx.Commit()
- if err != nil {
- log.Errorln("Could not commit transaction: ", err)
- return tc.DBError, tc.SystemError
+ w.Header().Set("Content-Type", "application/json")
+ fmt.Fprintf(w, "%s", respBts)
}
- rollbackTransaction = false
- return nil, tc.NoError
}
+func (dss *TODeliveryServiceServer) readDSS(db *sqlx.DB, params
map[string]string, user auth.CurrentUser) (*tc.DeliveryServiceServerResponse,
[]error, tc.ApiErrorType) {
+ limitstr := params["limit"]
+ pagestr := params["page"]
+ orderby := params["deliveryService"]
+ limit := 20
+ offset := 1
+ page := 1
+ var err error = nil
+
+ if limitstr != "" {
+ limit, err = strconv.Atoi(limitstr)
-func insertQuery() string {
- query := `INSERT INTO deliveryservice_server (
-deliveryservice,
-server) VALUES (
-:ds_id,
-:server_id) RETURNING deliveryservice, server, last_updated`
- return query
-}
+ if err != nil {
+ log.Errorf("limit parameter is not an integer")
+ return nil, []error{errors.New("limit parameter must be
an integer.")}, tc.SystemError
+ }
+ }
-func (dss *TODeliveryServiceServer) Read(db *sqlx.DB, params
map[string]string, user auth.CurrentUser) ([]interface{}, []error,
tc.ApiErrorType) {
- idstr, ok := params["id"]
+ if pagestr != "" {
+ offset, err = strconv.Atoi(pagestr)
+ page, err = strconv.Atoi(pagestr)
- if !ok {
- log.Errorf("Deliveryservice Server Id missing")
- return nil, []error{errors.New("Deliverservice id is
required.")}, tc.DataMissingError
+ if err != nil {
+ log.Errorf("page parameter is not an integer")
+ return nil, []error{errors.New("page parameter must be
an integer.")}, tc.SystemError
+ }
+
+ if offset > 0 {
+ offset -= 1
+ }
+
+ offset *= limit
}
- id, err := strconv.Atoi(idstr)
- if err != nil {
- log.Errorf("Deliveryservice Server Id is not an integer")
- return nil, []error{errors.New("Deliverservice id is not an
integer.")}, tc.SystemError
+ if orderby == "" {
+ orderby = "deliveryService"
}
- query := selectQuery()
+ query := selectQuery(orderby, strconv.Itoa(limit), strconv.Itoa(offset))
Review comment:
This is constructing the select query directly from the user input
`orderby`, which is a SQL Injection vulnerability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
