From a4274a8a0e89e82c7ee0ecf3c62dfd8187e4f208 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 23 Sep 2022 14:51:39 -0400 Subject: [PATCH] gopls: add diagnostics for non-stdlib vulnerabilities Show the vulnerabilities found by the runvulncheck codelens in the go.mod file using diagnostics. This uses a similar strategy to upgrade codelenses to store the vulnerabilities in the view so the diagnostics can be calculated at a later time. Change-Id: Ie9744712d9a7f8d78cbe3b54aa4cd3a380a304bc Reviewed-on: https://go-review.googlesource.com/c/tools/+/433537 Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro Reviewed-by: Robert Findley TryBot-Result: Gopher Robot Run-TryBot: Suzy Mueller --- gopls/internal/lsp/cache/session.go | 2 + gopls/internal/lsp/cache/view.go | 31 +++- gopls/internal/lsp/code_action.go | 1 + gopls/internal/lsp/command.go | 11 +- gopls/internal/lsp/diagnostics.go | 16 +++ gopls/internal/lsp/mod/diagnostics.go | 79 +++++++++- gopls/internal/lsp/source/view.go | 11 ++ .../misc/testdata/vulndb/golang.org/amod.json | 34 +++++ .../misc/testdata/vulndb/golang.org/bmod.json | 18 +++ gopls/internal/regtest/misc/vuln_test.go | 135 +++++++++++++++++- 10 files changed, 321 insertions(+), 17 deletions(-) create mode 100644 gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json create mode 100644 gopls/internal/regtest/misc/testdata/vulndb/golang.org/bmod.json diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index 5d40ff6be5..8a45ae24af 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -13,6 +13,7 @@ import ( "sync" "sync/atomic" + "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/progress" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/internal/event" @@ -238,6 +239,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, name: name, folder: folder, moduleUpgrades: map[span.URI]map[string]string{}, + vulns: map[span.URI][]command.Vuln{}, filesByURI: map[span.URI]*fileBase{}, filesByBase: map[string][]*fileBase{}, rootURI: root, diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index 025419d71d..af4ab9afbb 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -24,6 +24,7 @@ import ( "golang.org/x/mod/semver" exec "golang.org/x/sys/execabs" "golang.org/x/tools/go/packages" + "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/internal/bug" @@ -60,6 +61,8 @@ type View struct { // Each modfile has a map of module name to upgrade version. moduleUpgrades map[span.URI]map[string]string + vulns map[span.URI][]command.Vuln + // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files filesByURI map[span.URI]*fileBase @@ -1001,18 +1004,18 @@ func (v *View) IsGoPrivatePath(target string) bool { return globsMatchPath(v.goprivate, target) } -func (v *View) ModuleUpgrades(uri span.URI) map[string]string { +func (v *View) ModuleUpgrades(modfile span.URI) map[string]string { v.mu.Lock() defer v.mu.Unlock() upgrades := map[string]string{} - for mod, ver := range v.moduleUpgrades[uri] { + for mod, ver := range v.moduleUpgrades[modfile] { upgrades[mod] = ver } return upgrades } -func (v *View) RegisterModuleUpgrades(uri span.URI, upgrades map[string]string) { +func (v *View) RegisterModuleUpgrades(modfile span.URI, upgrades map[string]string) { // Return early if there are no upgrades. if len(upgrades) == 0 { return @@ -1021,10 +1024,10 @@ func (v *View) RegisterModuleUpgrades(uri span.URI, upgrades map[string]string) v.mu.Lock() defer v.mu.Unlock() - m := v.moduleUpgrades[uri] + m := v.moduleUpgrades[modfile] if m == nil { m = make(map[string]string) - v.moduleUpgrades[uri] = m + v.moduleUpgrades[modfile] = m } for mod, ver := range upgrades { m[mod] = ver @@ -1035,7 +1038,23 @@ func (v *View) ClearModuleUpgrades(modfile span.URI) { v.mu.Lock() defer v.mu.Unlock() - v.moduleUpgrades[modfile] = nil + delete(v.moduleUpgrades, modfile) +} + +func (v *View) Vulnerabilities(modfile span.URI) []command.Vuln { + v.mu.Lock() + defer v.mu.Unlock() + + vulns := make([]command.Vuln, len(v.vulns[modfile])) + copy(vulns, v.vulns[modfile]) + return vulns +} + +func (v *View) SetVulnerabilities(modfile span.URI, vulns []command.Vuln) { + v.mu.Lock() + defer v.mu.Unlock() + + v.vulns[modfile] = vulns } // Copied from diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go index dea83f5bdd..11013b5bb1 100644 --- a/gopls/internal/lsp/code_action.go +++ b/gopls/internal/lsp/code_action.go @@ -81,6 +81,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } + // TODO(suzmue): get upgrades code actions from vulnerabilities. quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, append(diags, udiags...)) if err != nil { return nil, err diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index 811307c1ff..5ded346760 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -207,10 +207,12 @@ func (c *commandHandler) ResetGoModDiagnostics(ctx context.Context, uri command. forURI: uri.URI, }, func(ctx context.Context, deps commandDeps) error { deps.snapshot.View().ClearModuleUpgrades(uri.URI.SpanURI()) - // Clear all diagnostics coming from the upgrade check source. + deps.snapshot.View().SetVulnerabilities(uri.URI.SpanURI(), nil) + // Clear all diagnostics coming from the upgrade check source and vulncheck. // This will clear the diagnostics in all go.mod files, but they // will be re-calculated when the snapshot is diagnosed again. c.s.clearDiagnosticSource(modCheckUpgradesSource) + c.s.clearDiagnosticSource(modVulncheckSource) // Re-diagnose the snapshot to remove the diagnostics. c.s.diagnoseSnapshot(deps.snapshot, nil, false) @@ -889,10 +891,9 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc return fmt.Errorf("failed to parse govulncheck output: %v", err) } - // TODO(jamalc,suzmue): convert the results to diagnostics & code actions. - // Or should we just write to a file (*.vulncheck.json) or text format - // and send "Show Document" request? If *.vulncheck.json is open, - // VSCode Go extension will open its custom editor. + deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), vulns.Vuln) + c.s.diagnoseSnapshot(deps.snapshot, nil, false) + set := make(map[string]bool) for _, v := range vulns.Vuln { if len(v.CallStackSummaries) > 0 { diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go index c00eb4206e..3efcaffcda 100644 --- a/gopls/internal/lsp/diagnostics.go +++ b/gopls/internal/lsp/diagnostics.go @@ -38,6 +38,7 @@ const ( orphanedSource workSource modCheckUpgradesSource + modVulncheckSource ) // A diagnosticReport holds results for a single diagnostic source. @@ -256,6 +257,21 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn } s.storeDiagnostics(snapshot, id.URI, modCheckUpgradesSource, diags) } + vulnerabilityReports, vulnErr := mod.VulnerabilityDiagnostics(ctx, snapshot) + if ctx.Err() != nil { + log.Trace.Log(ctx, "diagnose cancelled") + return + } + if vulnErr != nil { + event.Error(ctx, "warning: checking vulnerabilities", vulnErr, tag.Directory.Of(snapshot.View().Folder().Filename()), tag.Snapshot.Of(snapshot.ID())) + } + for id, diags := range vulnerabilityReports { + if id.URI == "" { + event.Error(ctx, "missing URI for module diagnostics", fmt.Errorf("empty URI"), tag.Directory.Of(snapshot.View().Folder().Filename())) + continue + } + s.storeDiagnostics(snapshot, id.URI, modVulncheckSource, diags) + } // Diagnose the go.work file, if it exists. workReports, workErr := work.Diagnostics(ctx, snapshot) diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go index 1efbfdd7de..0405fc7687 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -10,6 +10,7 @@ import ( "context" "fmt" + "golang.org/x/mod/semver" "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" @@ -17,6 +18,7 @@ import ( "golang.org/x/tools/internal/event/tag" ) +// Diagnostics returns diagnostics for the modules in the workspace. func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) { ctx, done := event.Start(ctx, "mod.Diagnostics", tag.Snapshot.Of(snapshot.ID())) defer done() @@ -24,6 +26,8 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.Vers return collectDiagnostics(ctx, snapshot, ModDiagnostics) } +// UpgradeDiagnostics returns upgrade diagnostics for the modules in the +// workspace with known upgrades. func UpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) { ctx, done := event.Start(ctx, "mod.UpgradeDiagnostics", tag.Snapshot.Of(snapshot.ID())) defer done() @@ -31,6 +35,15 @@ func UpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot) (map[sour return collectDiagnostics(ctx, snapshot, ModUpgradeDiagnostics) } +// VulnerabilityDiagnostics returns vulnerability diagnostics for the active modules in the +// workspace with known vulnerabilites. +func VulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) { + ctx, done := event.Start(ctx, "mod.VulnerabilityDiagnostics", tag.Snapshot.Of(snapshot.ID())) + defer done() + + return collectDiagnostics(ctx, snapshot, ModVulnerabilityDiagnostics) +} + func collectDiagnostics(ctx context.Context, snapshot source.Snapshot, diagFn func(context.Context, source.Snapshot, source.FileHandle) ([]*source.Diagnostic, error)) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) { reports := make(map[source.VersionedFileIdentity][]*source.Diagnostic) for _, uri := range snapshot.ModFiles() { @@ -103,10 +116,12 @@ func ModDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.Fil func ModUpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) (upgradeDiagnostics []*source.Diagnostic, err error) { pm, err := snapshot.ParseMod(ctx, fh) if err != nil { - if pm == nil || len(pm.ParseErrors) == 0 { - return nil, err + // Don't return an error if there are parse error diagnostics to be shown, but also do not + // continue since we won't be able to show the upgrade diagnostics. + if pm != nil && len(pm.ParseErrors) != 0 { + return nil, nil } - return nil, nil + return nil, err } upgrades := snapshot.View().ModuleUpgrades(fh.URI()) @@ -141,3 +156,61 @@ func ModUpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot, fh sou return upgradeDiagnostics, nil } + +// ModVulnerabilityDiagnostics adds diagnostics for vulnerabilities in individual modules +// if the vulnerability is recorded in the view. +func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) (vulnDiagnostics []*source.Diagnostic, err error) { + pm, err := snapshot.ParseMod(ctx, fh) + if err != nil { + // Don't return an error if there are parse error diagnostics to be shown, but also do not + // continue since we won't be able to show the vulnerability diagnostics. + if pm != nil && len(pm.ParseErrors) != 0 { + return nil, nil + } + return nil, err + } + + vs := snapshot.View().Vulnerabilities(fh.URI()) + // TODO(suzmue): should we just store the vulnerabilities like this? + vulns := make(map[string][]command.Vuln) + for _, v := range vs { + vulns[v.ModPath] = append(vulns[v.ModPath], v) + } + + for _, req := range pm.File.Require { + vulnList, ok := vulns[req.Mod.Path] + if !ok { + continue + } + rng, err := source.LineToRange(pm.Mapper, fh.URI(), req.Syntax.Start, req.Syntax.End) + if err != nil { + return nil, err + } + for _, v := range vulnList { + // Only show the diagnostic if the vulnerability was calculated + // for the module at the current version. + if semver.Compare(req.Mod.Version, v.CurrentVersion) != 0 { + continue + } + + severity := protocol.SeverityInformation + if len(v.CallStacks) > 0 { + severity = protocol.SeverityWarning + } + + vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{ + URI: fh.URI(), + Range: rng, + Severity: severity, + Source: source.Vulncheck, + Code: v.ID, + CodeHref: v.URL, + // TODO(suzmue): replace the newlines in v.Details to allow the editor to handle formatting. + Message: v.Details, + }) + } + + } + + return vulnDiagnostics, nil +} diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index 2565087336..8b1dbdbcf3 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -21,6 +21,7 @@ import ( "golang.org/x/mod/module" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" + "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/imports" @@ -269,6 +270,15 @@ type View interface { // ClearModuleUpgrades clears all upgrades for the modules in modfile. ClearModuleUpgrades(modfile span.URI) + // Vulnerabilites returns known vulnerabilities for the given modfile. + // TODO(suzmue): replace command.Vuln with a different type, maybe + // https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck/govulnchecklib#Summary? + Vulnerabilities(modfile span.URI) []command.Vuln + + // SetVulnerabilities resets the list of vulnerabilites that exists for the given modules + // required by modfile. + SetVulnerabilities(modfile span.URI, vulnerabilities []command.Vuln) + // FileKind returns the type of a file FileKind(FileHandle) FileKind } @@ -638,6 +648,7 @@ const ( ModTidyError DiagnosticSource = "go mod tidy" OptimizationDetailsError DiagnosticSource = "optimizer details" UpgradeNotification DiagnosticSource = "upgrade available" + Vulncheck DiagnosticSource = "govulncheck" TemplateError DiagnosticSource = "template" WorkFileError DiagnosticSource = "go.work file" ) diff --git a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json new file mode 100644 index 0000000000..b6790bf29e --- /dev/null +++ b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json @@ -0,0 +1,34 @@ +[ + { + "id":"GO-2022-01", + "details": "vuln in amod", + "affected":[ + { + "package":{"name":"golang.org/amod","ecosystem":"Go"}, + "ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.4"}, {"introduced": "1.1.2"}]}], + "ecosystem_specific":{ + "imports":[{"path":"golang.org/amod/avuln","symbols":["VulnData.Vuln1", "VulnData.Vuln2"]}] + } + } + ], + "references":[ + {"type":"href","url":"pkg.go.dev/vuln/GO-2022-01"} + ] + }, + { + "id":"GO-2022-03", + "details": "unaffecting vulnerability", + "affected":[ + { + "package":{"name":"golang.org/amod","ecosystem":"Go"}, + "ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.4"}]}], + "ecosystem_specific":{ + "imports":[{"path":"golang.org/amod/avuln","symbols":["nonExisting"]}] + } + } + ], + "references":[ + {"type":"href","url":"pkg.go.dev/vuln/GO-2022-03"} + ] + } +] \ No newline at end of file diff --git a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/bmod.json b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/bmod.json new file mode 100644 index 0000000000..b08b20d256 --- /dev/null +++ b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/bmod.json @@ -0,0 +1,18 @@ +[ + { + "id":"GO-2022-99", + "details": "vuln in bmod", + "affected":[ + { + "package":{"name":"golang.org/bmod","ecosystem":"Go"}, + "ranges":[{"type":"SEMVER"}], + "ecosystem_specific":{ + "imports":[{"path":"golang.org/bmod/bvuln","symbols":["Vuln"]}] + } + } + ], + "references":[ + {"type":"href","url":"pkg.go.dev/vuln/GO-2022-03"} + ] + } +] diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go index b17a279141..f05a959470 100644 --- a/gopls/internal/regtest/misc/vuln_test.go +++ b/gopls/internal/regtest/misc/vuln_test.go @@ -6,6 +6,7 @@ package misc import ( "os" + "path" "path/filepath" "testing" @@ -45,7 +46,7 @@ package foo }) } -func TestRunVulncheckExp(t *testing.T) { +func TestRunVulncheckExpStd(t *testing.T) { testenv.NeedsGo1Point(t, 18) const files = ` -- go.mod -- @@ -70,14 +71,14 @@ func main() { WithOptions( EnvVars{ // Let the analyzer read vulnerabilities data from the testdata/vulndb. - "GOVULNDB": "file://" + filepath.Join(cwd, "testdata", "vulndb"), + "GOVULNDB": "file://" + path.Join(filepath.ToSlash(cwd), "testdata", "vulndb"), // When fetchinging stdlib package vulnerability info, // behave as if our go version is go1.18 for this testing. // The default behavior is to run `go env GOVERSION` (which isn't mutable env var). // See gopls/internal/vulncheck.goVersion // which follows the convention used in golang.org/x/vuln/cmd/govulncheck. "GOVERSION": "go1.18", - "_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true", + "_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true", // needed to run `gopls vulncheck`. }, Settings{ "codelenses": map[string]bool{ @@ -115,3 +116,131 @@ func main() { ) }) } + +const workspace1 = ` +-- go.mod -- +module golang.org/entry + +go 1.18 + +require golang.org/cmod v1.1.3 + +require ( + golang.org/amod v1.1.3 // indirect + golang.org/bmod v0.5.0 // indirect +) +-- go.sum -- +golang.org/amod v1.1.3 h1:E9ohW9ayc6iCFrT/VNq8tCI4hgYM+tEbo8txbtbyS3o= +golang.org/amod v1.1.3/go.mod h1:yvny5/2OtYFomKt8ax+WJGvN6pfN1pqjGnn7DQLUi6E= +golang.org/bmod v0.5.0 h1:0kt1EI53298Ta9w4RPEAzNUQjtDoHUA6cc0c7Rwxhlk= +golang.org/bmod v0.5.0/go.mod h1:f6o+OhF66nz/0BBc/sbCsshyPRKMSxZIlG50B/bsM4c= +golang.org/cmod v1.1.3 h1:PJ7rZFTk7xGAunBRDa0wDe7rZjZ9R/vr1S2QkVVCngQ= +golang.org/cmod v1.1.3/go.mod h1:eCR8dnmvLYQomdeAZRCPgS5JJihXtqOQrpEkNj5feQA= +-- x/x.go -- +package x + +import ( + "golang.org/cmod/c" + "golang.org/entry/y" +) + +func X() { + c.C1().Vuln1() // vuln use: X -> Vuln1 +} + +func CallY() { + y.Y() // vuln use: CallY -> y.Y -> bvuln.Vuln +} + +-- y/y.go -- +package y + +import "golang.org/cmod/c" + +func Y() { + c.C2()() // vuln use: Y -> bvuln.Vuln +} +` + +const proxy1 = ` +-- golang.org/cmod@v1.1.3/go.mod -- +module golang.org/cmod + +go 1.12 +-- golang.org/cmod@v1.1.3/c/c.go -- +package c + +import ( + "golang.org/amod/avuln" + "golang.org/bmod/bvuln" +) + +type I interface { + Vuln1() +} + +func C1() I { + v := avuln.VulnData{} + v.Vuln2() // vuln use + return v +} + +func C2() func() { + return bvuln.Vuln +} +-- golang.org/amod@v1.1.3/go.mod -- +module golang.org/amod + +go 1.14 +-- golang.org/amod@v1.1.3/avuln/avuln.go -- +package avuln + +type VulnData struct {} +func (v VulnData) Vuln1() {} +func (v VulnData) Vuln2() {} +-- golang.org/bmod@v0.5.0/go.mod -- +module golang.org/bmod + +go 1.14 +-- golang.org/bmod@v0.5.0/bvuln/bvuln.go -- +package bvuln + +func Vuln() { + // something evil +} +` + +func TestRunVulncheckExp(t *testing.T) { + testenv.NeedsGo1Point(t, 18) + + cwd, _ := os.Getwd() + WithOptions( + ProxyFiles(proxy1), + EnvVars{ + // Let the analyzer read vulnerabilities data from the testdata/vulndb. + "GOVULNDB": "file://" + path.Join(filepath.ToSlash(cwd), "testdata", "vulndb"), + // When fetching stdlib package vulnerability info, + // behave as if our go version is go1.18 for this testing. + // The default behavior is to run `go env GOVERSION` (which isn't mutable env var). + // See gopls/internal/vulncheck.goVersion + // which follows the convention used in golang.org/x/vuln/cmd/govulncheck. + "GOVERSION": "go1.18", + "_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true", // needed to run `gopls vulncheck`. + }, + Settings{ + "codelenses": map[string]bool{ + "run_vulncheck_exp": true, + }, + }, + ).Run(t, workspace1, func(t *testing.T, env *Env) { + env.OpenFile("go.mod") + env.ExecuteCodeLensCommand("go.mod", command.RunVulncheckExp) + + env.Await( + CompletedWork("govulncheck", 1, true), + ShownMessage("Found"), + env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/amod`, "vuln in amod"), + env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/bmod`, "vuln in bmod"), + ) + }) +}