Skip to content

Commit

Permalink
unify validate-prefix & validate-objname; count list-objects err-s
Browse files Browse the repository at this point in the history 
* add ErrInvalidPrefix (type-code)
* refactor and micro-optimize validate-* helpers; unify
* move objname validation to proxies
* proxies to (also) count err.list.n
* refactor ver-changed, obj-move

Signed-off-by: Alex Aizman <alex.aizman@gmail.com>
  • Loading branch information
@alex-aizman
alex-aizman committed Sep 30, 2024
1 parent b76efec commit 5789273
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 114 deletions.
17 changes: 0 additions & 17 deletions ais/htrun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1395,23 +1395,6 @@ func (h *htrun) writeErrAct(w http.ResponseWriter, r *http.Request, action strin
cmn.FreeHterr(err)
}

func (h *htrun) writeErrActf(w http.ResponseWriter, r *http.Request, action string,
format string, a ...any) {
detail := fmt.Sprintf(format, a...)
err := cmn.InitErrHTTP(r, fmt.Errorf("invalid action %q: %s", action, detail), 0)
h.writeErr(w, r, err)
cmn.FreeHterr(err)
}

// also, validatePrefix
func (h *htrun) isValidObjname(w http.ResponseWriter, r *http.Request, name string) bool {
if err := cmn.ValidateObjName(name); err != nil {
h.writeErr(w, r, err)
return false
}
return true
}

// health client
func (h *htrun) reqHealth(si *meta.Snode, tout time.Duration, q url.Values, smap *smapX, retry bool) ([]byte, int, error) {
var (
Expand Down
146 changes: 92 additions & 54 deletions ais/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,6 @@ func (p *proxy) httpbckget(w http.ResponseWriter, r *http.Request, dpq *dpq) {
var (
msg *apc.ActMsg
bckName string
qbck *cmn.QueryBcks
)
apiItems, err := p.parseURL(w, r, apc.URLPathBuckets.L, 0, true)
if err != nil {
Expand All @@ -617,8 +616,10 @@ func (p *proxy) httpbckget(w http.ResponseWriter, r *http.Request, dpq *dpq) {
p.writeErr(w, r, err)
return
}
if qbck, err = newQbckFromQ(bckName, nil, dpq); err != nil {
p.writeErr(w, r, err)

qbck, errV := newQbckFromQ(bckName, nil, dpq)
if errV != nil {
p.writeErr(w, r, errV)
return
}

Expand Down Expand Up @@ -680,12 +681,13 @@ func (p *proxy) httpbckget(w http.ResponseWriter, r *http.Request, dpq *dpq) {
lsmsg apc.LsoMsg
bck = meta.CloneBck((*cmn.Bck)(qbck))
)
if err = cos.MorphMarshal(msg.Value, &lsmsg); err != nil {
if err := cos.MorphMarshal(msg.Value, &lsmsg); err != nil {
p.writeErrf(w, r, cmn.FmtErrMorphUnmarshal, p.si, msg.Action, msg.Value, err)
return
}
if lsmsg.Prefix != "" && strings.Contains(lsmsg.Prefix, "../") {
p.writeErrf(w, r, "bad list-objects request: invalid prefix %q", lsmsg.Prefix)
if err := cmn.ValidatePrefix("bad list-objects request", lsmsg.Prefix); err != nil {
p.statsT.IncErr(stats.ErrListCount)
p.writeErr(w, r, err)
return
}
bckArgs := bctx{p: p, w: w, r: r, msg: msg, perms: apc.AceObjLIST, bck: bck, dpq: dpq}
Expand All @@ -700,9 +702,12 @@ func (p *proxy) httpbckget(w http.ResponseWriter, r *http.Request, dpq *dpq) {
}

// do
if bck, err = bckArgs.initAndTry(); err == nil {
p.listObjects(w, r, bck, msg /*amsg*/, &lsmsg)
bck, errN := bckArgs.initAndTry()
if errN != nil {
p.statsT.IncErr(stats.ErrListCount)
return
}
p.listObjects(w, r, bck, msg /*amsg*/, &lsmsg)
}

// GET /v1/objects/bucket-name/object-name
Expand Down Expand Up @@ -737,6 +742,11 @@ func (p *proxy) httpobjget(w http.ResponseWriter, r *http.Request, origURLBck ..
p.statsT.IncErr(stats.ErrGetCount)
return
}
if err := cmn.ValidOname(objName); err != nil {
p.statsT.IncErr(stats.ErrGetCount)
p.writeErr(w, r, err)
return
}

started := time.Now()

Expand All @@ -763,19 +773,21 @@ func (p *proxy) httpobjget(w http.ResponseWriter, r *http.Request, origURLBck ..
func (p *proxy) httpobjput(w http.ResponseWriter, r *http.Request, apireq *apiRequest) {
var (
nodeID string
verb = "PUT"
errcnt = stats.ErrPutCount
perms apc.AccessAttrs
scnt = stats.PutCount
perms = apc.AcePUT
)
// 1. request
if err := p.parseReq(w, r, apireq); err != nil {
return
}
appendTyProvided := apireq.dpq.apnd.ty != "" // apc.QparamAppendType
if !appendTyProvided {
perms = apc.AcePUT
} else {
if appendTyProvided {
verb = "APPEND"
perms = apc.AceAPPEND
errcnt = stats.ErrAppendCount
scnt = stats.AppendCount
if apireq.dpq.apnd.hdl != "" {
items, err := preParse(apireq.dpq.apnd.hdl) // apc.QparamAppendHandle
if err != nil {
Expand Down Expand Up @@ -811,6 +823,11 @@ func (p *proxy) httpobjput(w http.ResponseWriter, r *http.Request, apireq *apiRe
objName = apireq.items[1]
netPub = cmn.NetPublic
)
if err := cmn.ValidOname(objName); err != nil {
p.statsT.IncErr(errcnt)
p.writeErr(w, r, err)
return
}
if nodeID == "" {
tsi, netPub, err = smap.HrwMultiHome(bck.MakeUname(objName))
if err != nil {
Expand All @@ -821,18 +838,14 @@ func (p *proxy) httpobjput(w http.ResponseWriter, r *http.Request, apireq *apiRe
} else {
if tsi = smap.GetTarget(nodeID); tsi == nil {
p.statsT.IncErr(errcnt)
err = &errNodeNotFound{"PUT failure:", nodeID, p.si, smap}
err = &errNodeNotFound{verb + " failure:", nodeID, p.si, smap}
p.writeErr(w, r, err)
return
}
}

// verbose
if cmn.Rom.FastV(5, cos.SmoduleAIS) {
verb := "PUT"
if appendTyProvided {
verb = "APPEND"
}
var s string
if bck.Props.Mirror.Enabled {
s = " (put-mirror)"
Expand All @@ -844,11 +857,7 @@ func (p *proxy) httpobjput(w http.ResponseWriter, r *http.Request, apireq *apiRe
http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect)

// 4. stats
if !appendTyProvided {
p.statsT.Inc(stats.PutCount)
} else {
p.statsT.Inc(stats.AppendCount)
}
p.statsT.Inc(scnt)
}

// DELETE /v1/objects/bucket-name/object-name
Expand All @@ -865,6 +874,11 @@ func (p *proxy) httpobjdelete(w http.ResponseWriter, r *http.Request) {
if err != nil {
return
}
if err := cmn.ValidOname(objName); err != nil {
p.statsT.IncErr(stats.ErrDeleteCount)
p.writeErr(w, r, err)
return
}
smap := p.owner.smap.get()
tsi, err := smap.HrwName2T(bck.MakeUname(objName))
if err != nil {
Expand Down Expand Up @@ -1614,17 +1628,9 @@ func crerrStatus(err error) (ecode int) {
func (p *proxy) listObjects(w http.ResponseWriter, r *http.Request, bck *meta.Bck, amsg *apc.ActMsg, lsmsg *apc.LsoMsg) {
// LsVerChanged a.k.a. '--check-versions' limitations
if lsmsg.IsFlagSet(apc.LsVerChanged) {
const a = "cannot perform remote versions check"
if !bck.HasVersioningMD() {
p.writeErrMsg(w, r, a+": bucket "+bck.Cname("")+" does not provide (remote) versioning info")
return
}
if lsmsg.IsFlagSet(apc.LsNameOnly) || lsmsg.IsFlagSet(apc.LsNameSize) {
p.writeErrMsg(w, r, a+": flag 'LsVerChanged' is incompatible with 'LsNameOnly', 'LsNameSize'")
return
}
if !lsmsg.WantProp(apc.GetPropsCustom) {
p.writeErrf(w, r, a+" without listing %q (object property)", apc.GetPropsCustom)
if err := _checkVerChanged(bck, lsmsg); err != nil {
p.statsT.IncErr(stats.ErrListCount)
p.writeErr(w, r, err)
return
}
}
Expand All @@ -1651,6 +1657,7 @@ func (p *proxy) listObjects(w http.ResponseWriter, r *http.Request, bck *meta.Bc
beg := mono.NanoTime()
lst, err := p.lsPage(bck, amsg, lsmsg, r.Header, p.owner.smap.get())
if err != nil {
p.statsT.IncErr(stats.ErrListCount)
p.writeErr(w, r, err)
return
}
Expand All @@ -1676,6 +1683,20 @@ func (p *proxy) listObjects(w http.ResponseWriter, r *http.Request, bck *meta.Bc
lst = nil
}

func _checkVerChanged(bck *meta.Bck, lsmsg *apc.LsoMsg) error {
const a = "cannot perform remote versions check"
if !bck.HasVersioningMD() {
return errors.New(a + ": bucket " + bck.Cname("") + " does not provide (remote) versioning info")
}
if lsmsg.IsFlagSet(apc.LsNameOnly) || lsmsg.IsFlagSet(apc.LsNameSize) {
return errors.New(a + ": flag 'LsVerChanged' is incompatible with 'LsNameOnly', 'LsNameSize'")
}
if !lsmsg.WantProp(apc.GetPropsCustom) {
return fmt.Errorf(a+" without listing %q (object property)", apc.GetPropsCustom)
}
return nil
}

// one page; common code (native, s3 api)
func (p *proxy) lsPage(bck *meta.Bck, amsg *apc.ActMsg, lsmsg *apc.LsoMsg, hdr http.Header, smap *smapX) (*cmn.LsoRes, error) {
var (
Expand Down Expand Up @@ -1811,27 +1832,18 @@ func (p *proxy) httpobjpost(w http.ResponseWriter, r *http.Request, apireq *apiR
switch msg.Action {
case apc.ActRenameObject:
if err := p.checkAccess(w, r, bck, apc.AceObjMOVE); err != nil {
p.statsT.IncErr(stats.ErrRenameCount)
return
}
if bck.IsRemote() {
p.writeErrActf(w, r, msg.Action, "not supported for remote buckets (%s)", bck)
return
}
if bck.Props.EC.Enabled {
p.writeErrActf(w, r, msg.Action, "not supported for erasure-coded buckets (%s)", bck)
return
}
objName, objNameTo := apireq.items[1], msg.Name
if objName == objNameTo {
p.writeErrMsg(w, r, "cannot rename "+bck.Cname(objName)+" to self, nothing to do")
return
}
if !p.isValidObjname(w, r, objNameTo) {
return
if err := _checkObjMv(bck, msg, apireq); err != nil {
p.statsT.IncErr(stats.ErrRenameCount)
p.writeErr(w, r, err)
}
p.redirectObjAction(w, r, bck, apireq.items[1], msg)
p.redirectAction(w, r, bck, apireq.items[1], msg)
p.statsT.Inc(stats.RenameCount)
case apc.ActPromote:
if err := p.checkAccess(w, r, bck, apc.AcePromote); err != nil {
p.statsT.IncErr(stats.ErrRenameCount)
return
}
// ActionMsg.Name is the source
Expand Down Expand Up @@ -1866,6 +1878,7 @@ func (p *proxy) httpobjpost(w http.ResponseWriter, r *http.Request, apireq *apiR
writeXid(w, xid)
}
case apc.ActBlobDl:
// TODO: add stats.GetBlobCount and *ErrCount
if err := p.checkAccess(w, r, bck, apc.AccessRW); err != nil {
return
}
Expand All @@ -1874,12 +1887,34 @@ func (p *proxy) httpobjpost(w http.ResponseWriter, r *http.Request, apireq *apiR
return
}
objName := msg.Name
p.redirectObjAction(w, r, bck, objName, msg)
p.redirectAction(w, r, bck, objName, msg)
default:
p.writeErrAct(w, r, msg.Action)
}
}

func _checkObjMv(bck *meta.Bck, msg *apc.ActMsg, apireq *apiRequest) error {
if bck.IsRemote() {
err := fmt.Errorf("invalid action %q: not supported for remote buckets (%s)", msg.Action, bck)
return cmn.NewErrUnsuppErr(err)
}
if bck.Props.EC.Enabled {
err := fmt.Errorf("invalid action %q: not supported for erasure-coded buckets (%s)", msg.Action, bck)
return cmn.NewErrUnsuppErr(err)
}
objName, objNameTo := apireq.items[1], msg.Name
if err := cmn.ValidOname(objName); err != nil {
return err
}
if err := cmn.ValidateOname(objNameTo); err != nil {
return err
}
if objName == objNameTo {
return fmt.Errorf("cannot rename %s to self, nothing to do", bck.Cname(objName))
}
return nil
}

// HEAD /v1/buckets/bucket-name[/prefix]
// with additional preparsing step to support api.GetBucketInfo prefix (as in: ais ls --summary)
func (p *proxy) httpbckhead(w http.ResponseWriter, r *http.Request, apireq *apiRequest) {
Expand Down Expand Up @@ -2125,6 +2160,10 @@ func (p *proxy) httpobjpatch(w http.ResponseWriter, r *http.Request) {
if err != nil {
return
}
if err := cmn.ValidOname(objName); err != nil {
p.writeErr(w, r, err)
return
}
smap := p.owner.smap.get()
si, err := smap.HrwName2T(bck.MakeUname(objName))
if err != nil {
Expand Down Expand Up @@ -2425,7 +2464,8 @@ func (p *proxy) lsObjsR(bck *meta.Bck, lsmsg *apc.LsoMsg, hdr http.Header, smap
return cmn.MergeLso(resLists, lsmsg, 0), nil
}

func (p *proxy) redirectObjAction(w http.ResponseWriter, r *http.Request, bck *meta.Bck, objName string, msg *apc.ActMsg) {
// http-redirect(with-json-message)
func (p *proxy) redirectAction(w http.ResponseWriter, r *http.Request, bck *meta.Bck, objName string, msg *apc.ActMsg) {
started := time.Now()
smap := p.owner.smap.get()
si, err := smap.HrwName2T(bck.MakeUname(objName))
Expand All @@ -2437,11 +2477,9 @@ func (p *proxy) redirectObjAction(w http.ResponseWriter, r *http.Request, bck *m
nlog.Infoln(msg.Action, bck.Cname(objName), "=>", si.StringEx())
}

// NOTE: Code 307 is the only way to http-redirect with the original JSON payload.
// 307 is the only way to http-redirect with the original JSON payload
redirectURL := p.redirectURL(r, si, started, cmn.NetIntraControl)
http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect)

p.statsT.Inc(stats.RenameCount)
}

func (p *proxy) listrange(method, bucket string, msg *apc.ActMsg, query url.Values) (xid string, err error) {
Expand Down
Loading

0 comments on commit 5789273

Please sign in to comment.