reflect.Value.Bytes() should only be called when reflect.Value.Kind()… #1346

Merged
teejays merged 1 commits from master into master 2019-07-05 02:59:58 +00:00
teejays commented 2019-07-05 01:37:13 +00:00 (Migrated from github.com)

Fix for issue #1345.

I noticed that in value2Interface function in session_convert.go, we are using reflect.Value.Bytes() function when the fieldType is reflect.Array OR reflect.Slice. However, go's reflect.Value.Bytes() only works when the value is of type reflect.Slice, and panics otherwise.

I would propose that change the if condition from:

if (k == reflect.Array || k == reflect.Slice) && (...) {
    // logic
}

to

if (k == reflect.Slice) && (...) {
    // logic
}

This PR fixes this.

Fix for issue [#1345](https://github.com/go-xorm/xorm/issues/1345). I noticed that in value2Interface function in session_convert.go, we are using reflect.Value.Bytes() function when the fieldType is reflect.Array OR reflect.Slice. However, go's reflect.Value.Bytes() only works when the value is of type reflect.Slice, and panics otherwise. I would propose that change the if condition from: ``` if (k == reflect.Array || k == reflect.Slice) && (...) { // logic } ``` to ``` if (k == reflect.Slice) && (...) { // logic } ``` This PR fixes this.
codecov-io commented 2019-07-05 01:41:44 +00:00 (Migrated from github.com)

Codecov Report

Merging #1346 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1346   +/-   ##
=======================================
  Coverage   57.48%   57.48%           
=======================================
  Files          44       44           
  Lines        7833     7833           
=======================================
  Hits         4503     4503           
  Misses       2773     2773           
  Partials      557      557
Impacted Files Coverage Δ
session_convert.go 22.29% <100%> (ø) ⬆️
xorm.go 66.66% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c80660...2484945. Read the comment docs.

# [Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=h1) Report > Merging [#1346](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=desc) into [master](https://codecov.io/gh/go-xorm/xorm/commit/4c806608ab1d39d93b8bdc2778acdf2c42735c04?src=pr&el=desc) will **not change** coverage. > The diff coverage is `100%`. [![Impacted file tree graph](https://codecov.io/gh/go-xorm/xorm/pull/1346/graphs/tree.svg?width=650&token=yB5nO1krEe&height=150&src=pr)](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #1346 +/- ## ======================================= Coverage 57.48% 57.48% ======================================= Files 44 44 Lines 7833 7833 ======================================= Hits 4503 4503 Misses 2773 2773 Partials 557 557 ``` | [Impacted Files](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [session\_convert.go](https://codecov.io/gh/go-xorm/xorm/pull/1346/diff?src=pr&el=tree#diff-c2Vzc2lvbl9jb252ZXJ0Lmdv) | `22.29% <100%> (ø)` | :arrow_up: | | [xorm.go](https://codecov.io/gh/go-xorm/xorm/pull/1346/diff?src=pr&el=tree#diff-eG9ybS5nbw==) | `66.66% <0%> (ø)` | :arrow_up: | ------ [Continue to review full report at Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=footer). Last update [4c80660...2484945](https://codecov.io/gh/go-xorm/xorm/pull/1346?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
teejays commented 2019-07-05 01:59:32 +00:00 (Migrated from github.com)

Do I need to do something to pass the WIP check? :/

Do I need to do something to pass the WIP check? :/
Sign in to join this conversation.
No description provided.