{"action":"created","issue":{"url":"https://api.github.com/repos/go-ini/ini/issues/191","repository_url":"https://api.github.com/repos/go-ini/ini","labels_url":"https://api.github.com/repos/go-ini/ini/issues/191/labels{/name}","comments_url":"https://api.github.com/repos/go-ini/ini/issues/191/comments","events_url":"https://api.github.com/repos/go-ini/ini/issues/191/events","html_url":"https://github.com/go-ini/ini/issues/191","id":441575569,"node_id":"MDU6SXNzdWU0NDE1NzU1Njk=","number":191,"title":"Panic when transform %(xxxxx)s value","user":{"login":"ryanyux","id":7455938,"node_id":"MDQ6VXNlcjc0NTU5Mzg=","avatar_url":"https://avatars1.githubusercontent.com/u/7455938?v=4","gravatar_id":"","url":"https://api.github.com/users/ryanyux","html_url":"https://github.com/ryanyux","followers_url":"https://api.github.com/users/ryanyux/followers","following_url":"https://api.github.com/users/ryanyux/following{/other_user}","gists_url":"https://api.github.com/users/ryanyux/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanyux/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanyux/subscriptions","organizations_url":"https://api.github.com/users/ryanyux/orgs","repos_url":"https://api.github.com/users/ryanyux/repos","events_url":"https://api.github.com/users/ryanyux/events{/privacy}","received_events_url":"https://api.github.com/users/ryanyux/received_events","type":"User","site_admin":false},"labels":[{"id":1623087609,"node_id":"MDU6TGFiZWwxNjIzMDg3NjA5","url":"https://api.github.com/repos/go-ini/ini/labels/kind:%20bug","name":"kind: bug","color":"d73a4a","default":false},{"id":1623087693,"node_id":"MDU6TGFiZWwxNjIzMDg3Njkz","url":"https://api.github.com/repos/go-ini/ini/labels/status:%20needs%20feedback","name":"status: needs feedback","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2019-05-08T06:41:01Z","updated_at":"2019-10-23T09:31:41Z","closed_at":null,"author_association":"NONE","body":"### Please give general description of the problem\r\n\r\nin gokpg.ini/ini.v1/key.go  \r\nfunc (k *Key) transformValue(val string) string\r\n\r\nWhen transform a %(xxxxx)s value, it's gonna to find the key xxxxx, then read key's value.\r\nBut if the key is not found, read key's value will cause a nil pointer panic\r\n\r\n### Please provide code snippets to reproduce the problem described above\r\n\r\n\t\tnk, err := k.s.GetKey(noption)\r\n\t\tif err != nil || k == nk {\r\n\t\t\t// Search again in default section.\r\n\t\t\tnk, _ = k.s.f.Section(\"\").GetKey(noption)\r\n\t\t}\r\n\r\n\t\t// Substitute by new value and take off leading '%(' and trailing ')s'.\r\n\t\tval = strings.Replace(val, vr, nk.value, -1)\r\n\r\n\r\n### Do you have any suggestion to fix the problem?\r\n\r\nif the key is not found in default section, the function can return an empty string.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/go-ini/ini/issues/comments/545356858","html_url":"https://github.com/go-ini/ini/issues/191#issuecomment-545356858","issue_url":"https://api.github.com/repos/go-ini/ini/issues/191","id":545356858,"node_id":"MDEyOklzc3VlQ29tbWVudDU0NTM1Njg1OA==","user":{"login":"j-vizcaino","id":4704917,"node_id":"MDQ6VXNlcjQ3MDQ5MTc=","avatar_url":"https://avatars0.githubusercontent.com/u/4704917?v=4","gravatar_id":"","url":"https://api.github.com/users/j-vizcaino","html_url":"https://github.com/j-vizcaino","followers_url":"https://api.github.com/users/j-vizcaino/followers","following_url":"https://api.github.com/users/j-vizcaino/following{/other_user}","gists_url":"https://api.github.com/users/j-vizcaino/gists{/gist_id}","starred_url":"https://api.github.com/users/j-vizcaino/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/j-vizcaino/subscriptions","organizations_url":"https://api.github.com/users/j-vizcaino/orgs","repos_url":"https://api.github.com/users/j-vizcaino/repos","events_url":"https://api.github.com/users/j-vizcaino/events{/privacy}","received_events_url":"https://api.github.com/users/j-vizcaino/received_events","type":"User","site_admin":false},"created_at":"2019-10-23T09:31:41Z","updated_at":"2019-10-23T09:31:41Z","author_association":"NONE","body":"Sorry for the unclear explanation, let me try again with an example.\r\n\r\n### Example\r\n\r\nConsider the following code:\r\n\r\n```go\r\nurl := cfg.Section(\"database\").Key(\"url\").String()\r\nif err := mydb.Connect(url); err != nil {\r\n  return fmt.Errorf(\"Database %s connection failed: %w\", url, err)\r\n}\r\n...\r\n```\r\n\r\nWith INI\r\n\r\n```\r\n[database]\r\nurl = %(main_db_url)s\r\n```\r\n\r\nNow, say that `url` holds an invalid interpolated string in the config.\r\n\r\n#### Option 1: returning the raw, non interpolated value\r\n\r\n* Error message would read `Database %(main_db_url)s connection failed: ...`\r\n* This is not great: the INI file is known to be invalid before even calling the DB connection call. This should have never reached the location in the code where the config value is even being used.\r\n* This can be dangerous, as the interpolation can be interpreted somehow by the function that receives the value (potential injection)  \r\n\r\n#### Option 2: returning an empty string\r\n\r\n* Error message would read `Database  connection failed: ...`\r\n* Note the double space here, since `url` is empty: this is not great for troubleshooting, as most readers would not realise the URL is empty and probably think the error log is useless, lacking context.\r\nHence my comment regarding using `%q` instead of `%s` in the `fmt.Errorf` call above to surround the `url` value with quotes. \r\n\r\n### Next?\r\n\r\nIMO, none of these are good solutions as they both have tradeoffs. The API, due to lazy loading it seems, makes it hard to report errors for strings keys, when interpolation is invalid.\r\n\r\nHere are the things that came to mind (I hope that helps):\r\n\r\n* Introduce `StrictString() (string, error)` and have the error bubble up to the caller. This implies users will need to update their code to handle that case properly\r\n* Have `GetKey` return an error if the value interpolation fails? This might be unexpected though 🤔 \r\n\r\nIf `StrictString` is the path to go, then `String` could return the non interpolated string. The user would probably be able to figure out the value is invalid.\r\n\r\nWDYT?"}}
{"actor":{"display_login":"j-vizcaino"}}