Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix/v0.45.2.1 #2

Open
wants to merge 269 commits into
base: master
Choose a base branch
from
Open

Hotfix/v0.45.2.1 #2

wants to merge 269 commits into from

Conversation

raghavTinker
Copy link
Collaborator

No description provided.

metabase-bot bot and others added 30 commits November 4, 2022 20:49
First seen in trivy report:
https://github.com/metabase/metabase/pull/26161/checks?check_run_id=9326286850

CVE:
https://avd.aquasec.com/nvd/cve-2022-40151

xstream: Xstream to serialise XML data was vulnerable to Denial of
Service attacks High
Package: com.fasterxml.woodstox:woodstox-core
Installed Version: 6.2.6
Vulnerability CVE-2022-40151
Severity: HIGH
Fixed Version: 5.4.0, 6.4.0

Bumping deps and comparing `clj -X:deps tree` shows the change only adds
the new dep top level and no new deps are brought in by the change.

```
❯ diff --unified deps deps-updated
--- deps	2022-11-07 08:43:21.000000000 -0600
+++ deps-updated	2022-11-07 08:49:56.000000000 -0600
@@ -9,6 +9,8 @@
   X org.slf4j/slf4j-api 1.7.25 :use-top
   X org.apache.logging.log4j/log4j-api 2.18.0 :use-top
   X org.apache.logging.log4j/log4j-core 2.18.0 :use-top
+com.fasterxml.woodstox/woodstox-core 6.4.0
+  . org.codehaus.woodstox/stax2-api 4.2.1
 joda-time/joda-time 2.10.13
 commons-codec/commons-codec 1.15
 weavejester/dependency 0.2.1
@@ -285,8 +287,7 @@
   . org.apache.santuario/xmlsec 2.3.0
     X org.slf4j/slf4j-api 1.7.32 :use-top
     X commons-codec/commons-codec 1.15 :use-top
-    . com.fasterxml.woodstox/woodstox-core 6.2.6
-      . org.codehaus.woodstox/stax2-api 4.2.1
+    X com.fasterxml.woodstox/woodstox-core 6.2.6 :use-top
     . jakarta.xml.bind/jakarta.xml.bind-api 2.3.3
       . jakarta.activation/jakarta.activation-api 1.2.2
   . org.opensaml/opensaml-xmlsec-api 3.4.6
```

Co-authored-by: dpsutton <[email protected]>
* add week function

* Update docs/questions/query-builder/expressions-list.md

Co-authored-by: Natalie <[email protected]>

* update copy

* formatting

Co-authored-by: Natalie <[email protected]>

Co-authored-by: Jeff Bruemmer <[email protected]>
Co-authored-by: Natalie <[email protected]>
…) (metabase#26282)

* Adding Tests for hasColumnSettings

* Updating Comment

Co-authored-by: Nick Fitzpatrick <[email protected]>
…ngtext (metabase#26223) (metabase#26287)

* Change the remaning columns that has text type in mysql,mariadb to
longtext

Co-authored-by: Ngoc Khuat <[email protected]>
…etabase#26026) (metabase#26307)

* Uses all text scorers in the final result

- instead of just the maximum one
- add tests

* add prefix scorer test + fix text-score-with

* linter fixes

* pass in number of results to find

* refactor test function

* fix linter by removing unused namespace: metabase.util

* limit arity of serialize to 3

* make oss-score and ee-score different things

- They were defined to be exactly the same, but should be different!
- Update some tests that broke when a test function was fixed

* remove extra let

* move rseq back out of sorted-take

* improve test feedback

* force weight of text based scorers always weigh 10

* handle 0 score/weights when normalizing scores

* add nil check

* fix more subtle test differences

* more test fiddling

- still test that :offset and :limit respect limits

* reuse bit->boolean from api collection

* clean up some tests

- filter -> remove
- replace some magic numbers
- revert to testing entire maps instead of names of sorted items

* add test, docstring, and weight

* sort ns requires

* responding to most of the review comments

* start our zero-score sum check with 0

* do not tokenize / normalize nil raw-search-string

* force equality in basic search test

* modify test to work in dev and test environments

* use display_name in results when appropriate

- This was looking for the _first_ column that had a non-zero score, but
actually we need to consider all relevant columns.
- Uses them to figure out if there is a display name, and if there is,
to use it.
- Coppied over the logic about showing :context from the prior approach

Co-authored-by: Bryan Maass <[email protected]>
* Upgrade `cypress/grep` library

* Update Cypress config

Co-authored-by: Nemanja Glumac <[email protected]>
* log errors caught during sync steps

* remove accidental extra parens

Co-authored-by: Noah Moss <[email protected]>
…etabase#26279) (metabase#26338)

* Add failing tests

* Fix failing tests

* Update shared/src/metabase/mbql/util.cljc

Co-authored-by: Ngoc Khuat <[email protected]>

* datetime-add and datetime subtract should annotate type by col type

* Fix infer-expression-type for datetime-add/subtract with second, minute, hour

* Undo last commit; they actually always return :type/DateTime

* Fix test based on last commit

* Undo unrelated refactor

* Only test drivers that support expressions

* Only test drivers that support expressions, again

* Update tests from legacy mbql

* Change infered-col-type to be a function again, not macro

* Fix test

Co-authored-by: Ngoc Khuat <[email protected]>

Co-authored-by: Cal Herries <[email protected]>
Co-authored-by: Ngoc Khuat <[email protected]>
)

* Fix stacked static chart render failure

This fixes metabase#20752 where static chart will fail to render stacked bar
charts when data contain gaps.

* Make the code easier to read

Co-authored-by: Mahatthana (Kelvin) Nomsawadi <[email protected]>
…e#26378)

* Migrate some driver tests to GitHub Actions

* Druid
* MariaDB 10.2
* MariaDB latest
* MongoDB 4.0
* MongoDB 5.0
* MongoDB latest
* MySQL 5.7
* Postgres 9.6
* Postgres latest
* Presto
* Spark
* SQLite
* SQL Server 2017

* Don't use Buildjet runner

* Don't run the driver tests for draft PRs

* Use Buildjet for Spark driver tests

* Use Buildjet for Druid driver tests

Co-authored-by: Ariya Hidayat <[email protected]>
nemanjaglumac and others added 30 commits December 27, 2022 22:34
…se#27445)

* Fix BigQuery parsing of `bigdecimal` results

* Avoid big changes to `ns` form

* Update tests

Co-authored-by: Cam Saul <[email protected]>
* Repro metabase#27427: Static-viz fails for unused returned column (metabase#27441)

* Repro metabase#19373: Pivot table returns wrong (distinct) row totals (metabase#27457)

* Repro metabase#18770: Post-aggregation filter disrupts drill-through (metabase#27459)

* Repro metabase#25250: Pivot Table doesn't show stand-alone values on collapsed sub-level grouping (metabase#27460)
…ro error (metabase#27518) (metabase#27535)

* Disallow axis ranges with the same min/max, which causes divide by zero error

The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough',
which is calculated by determining the percentage overlap the ranges share.

As the comparison is made, the first series in the list will always be compared with itself, and should have
1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series,
and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the
same value in every row.

With this change, these ranges are considered invalid, and we avoid the divide by zero.

* Unskip e2e repro

Co-authored-by: adam-james <[email protected]>
```
docker run \
       -p 3000:3000 \
       -v $PWD/my_log4j2.xml:/tmp/my_log4j2.xml \
       -e JAVA_OPTS=-Dlog4j.configurationFile=/tmp/my_log4j2.xml \
       metabase/metabase:v0.45.1
```

From metabase#27497

Note that this specifies `log4j.configurationFile` and not
"log4j2.configurationFile" that we set in our bootstrap.clj.

I'm not sure if this is supposed to work or not but we'll give it a
shot.

the Automatic Configuration section of
https://logging.apache.org/log4j/2.x/manual/configuration.html specifies
that:
1. Log4j will inspect the "log4j2.configurationFile" system property
and, if set, will attempt to load the configuration using the
ConfigurationFactory that matches the file extension. Note that this is
not restricted to a location on the local file system and may contain a
URL.

I'm not sure if that other property will work due to some other
mechanism or log4j2 has an undocumented back-compat mechanism.
…line/area/bar …" (metabase#27371)

* ensuring _raw prop is present on transformed series in line/area/bar charts (metabase#27357)

* backporting type dependencies

Co-authored-by: Nick Fitzpatrick <[email protected]>
…etabase#27593)

This e2e test failed in the 45 release branch because 46 (in progress on mater at time of writing) has an upgraded H2
driver, which was not backported.

This meant that the e2e test's expection of a graph.dimension of `EXTRACT(YEAR FROM CURRENT_TIMESTAMP)` would cause
the static-viz render to fail because the actual dimension would be `YEAR(CURRENT_TIMESTAMP())`.

The fix to the underlying problem is still valid, and this reproduction, with the correct dimension, tests the fix and
behaves correctly.
…etabase#27603)

* remove panning logic from map boundary calculation

* add map viz percy tests

* wait for map load in visual tests

Co-authored-by: Ryan Laurie <[email protected]>
…onth (metabase#27557) (metabase#27600)

Since the frontend sends a date-style string with various differences from how Java's Date Formatter expects things,
we need to post-process some bits. The 'D' -> 'd' string replacement was missed in the post processing fn, and so in
some cases it was missed altogether, causing this bug.

Existing tests in `metabase.pulse.render.datetime-test` and `metabase.pulse.render.table-test` should already catch
this issue. Prior to the change they would fail, but they now pass after adding this PR's changes.

Co-authored-by: adam-james <[email protected]>
…tabase#27567)

* Fix double aggregation not working on some fields.

This fix adds make sure the table field source is `fields` as this is
done in the previous version before we changed it in PR#25267 which
introduces the bug in issue#27462

* Add a reproduction

* Address review: Improve test readability

Co-authored-by: Mahatthana (Kelvin) Nomsawadi <[email protected]>
…ore than 2 items (metabase#27585) (metabase#27592)

* oxford comma for parameter values with more than 2 items

* refactor logic to use a better translated string and fix tests

Co-authored-by: Noah Moss <[email protected]>
…ams" (metabase#27571)

* Parse comma-separated list of numbers as URL filter params (metabase#27436)

* Trigger CI [ci nocache]

Co-authored-by: Gustavo Saiani <[email protected]>
Co-authored-by: Gustavo Saiani <[email protected]>
* temp

* Adding cypress test

* removing test

* unskipping test

Co-authored-by: Nick Fitzpatrick <[email protected]>
* restore aliases before annotating

* cleanup

* fix tests

* Don't add escaped->original if aliases have not changed

No need to walk and replace the aliases if they are identical. And in
that case, no need to keep a mapping of identical to identical. Not
super important but saves some time and complexity, and keeps other
tests passing since the presence of [:info :alias/escaped->original] in
the query caused them to trivially fail.

* oracle has a smaller limit and _expected_ mangled

previous testing behavior was "what happened" and not what should
happen. we fixed the bug but the "expect garbage" behavior was still
present

* Relax :alias/escaped->original schema

oracle tests use keywords for the alias

```clojure
{:alias/escaped->original
 {:test-data-venues--via-a763718f "test_data_venues__via__venue_id",
  :test-data-users--via--user-id "test_data_users__via__user_id"}}}
```

No idea why that is keyworded

* relax `:alias/escaped->original` schema

see previous commit
…e#27676)

* Fixed regex for latitude name type matching

* Grouping regexes by lon/lat

Co-authored-by: Mark Bastian <[email protected]>
* nit alignment

* point to var so changes are picked up

need to refresh this ns after changing the middleware since these add a
bit more helpful doc strings. But capture previous function value so
more annoying at repl

* Ensure MB_API_KEY is set for notify endpoint

Previously when the MB_API_KEY was unset:

```
❯ http POST "localhost:3000/api/notify/db/1?scan=schema"
HTTP/1.1 403 Forbidden
<other headers removed>

Forbidden
```

And now:

```
❯ http POST "localhost:3000/api/notify/db/1?scan=schema"
HTTP/1.1 403 Forbidden
<other headers removed>

MB_API_KEY is not set. See https://www.metabase.com/docs/latest/configuring-metabase/environment-variables#mb_api_key for details
```

An annoying thing in the tests. We set `"-Dmb.api.key=test-api-key"` in
the `:test` alias for CI, but not in regular dev mode. So annoyingly we
either have tests that would fail in the repl in regular dev, or use
`mt/with-temporary-setting-values [api-key "test-api-key"]` when writing
tests that only matter for local stuff. c'est la vie

* Translate error message

Using site locale because this in an `+apikey` context, not an `+auth`
context. The importance of that is that there is no logged in user so no
way to get a user locale, and it just falls back to the server locale
anyways. So lets just use that.

* Fix temp settings when they are env-based

An annoying bit here. `api-key` setting in the notify test is set in the
env in the `:test` alias.

```clojure
notify-test=> (mt/with-temporary-setting-values [api-key nil]
                (mw.auth/api-key))
"test-api-key" ;; where did the nil go?
```

But there are two bugs in `mt/with-temporary-setting-values`:

1. It gets the current env value and then temporarily sets it to the
current value. Note that `do-with-temp-env-var-value` is called with
`env-var-value` which is the current value in the environment. All of
this work for nothing

```
-    (if-let [env-var-value (and (not raw-setting?) (#'setting/env-var-value setting-k))]
-      (do-with-temp-env-var-value setting env-var-value thunk)
+    (if (and (not raw-setting?) (#'setting/env-var-value setting-k))
+      (do-with-temp-env-var-value setting-k value thunk)
```
So cannot possibly do what we want.

2. The second bug is that the pathway through
`mt/with-temporary-setting-values` did not convert `:api-key` to
`:mb-api-key`. So even if it put our new value in above, it would put
`:api-key nil` in the env, but when we look for it it would look for
`:mb-api-key` and we would not find it.

Thus the use of `setting-env-map-name`. Then clean up the few other
places that did it kinda right, but not necessarily. Seems the "correct"
way to do this is get the setting name, munge it, prepend with mb. The
other call sites were using variants of `(keyword (str "mb-" (name
setting-name)))` which is close but could miss the munging.

* Safely set env/env keys

There are two ways that we put things into the env/env so they appear as
environmentally set variables.

- explicitly with `mt/with-temp-env-var-value`. This simulates a user
setting these values and they are specified in the env way:
mb-email-smtp-port, "MB_DISABLE_SCHEDULER", etc. At the call site you
are aware that they are env related so get the mb prefix
- incidentally with `mt/with-temporary-setting-values`. Settings are
always referred to by their "nice" internal name like `api-key`,
etc. But if a setting is found to be an env variable, we override it in
env/env. But we have to make sure the key is set properly with an mb-
prefix.

Owing to this, we have to conditionally add the mb- prefix if not
already present.

* cleanup the constants a bit

* Fix some site-url tests

The tests themselves are testing useful stuff: site-url doesn't return a
broken value if it fails the verification when set as an env
variable. But the tests themselves were a bit nonsensical in their
setup:

```
@@ -62,21 +62,19 @@
 (deftest site-url-settings-normalize
   (testing "We should normalize `site-url` when set via env var we should still normalize it (metabase#9764)"
     (mt/with-temp-env-var-value [mb-site-url "localhost:3000/"]
-      (mt/with-temporary-setting-values [site-url nil]
-        (is (= "localhost:3000/"
-               (setting/get-value-of-type :string :site-url)))
-        (is (= "http://localhost:3000"
-               (public-settings/site-url)))))))
+      (is (= "localhost:3000/"
+             (setting/get-value-of-type :string :site-url)))
+      (is (= "http://localhost:3000"
+             (public-settings/site-url))))))
```

Why would it set an env-variable version [mb-site-url "localhost:3000/"]
then try to blank it out with `(mt/with-temporary-setting-values
[site-url nil])`.

Fixing the bug in how we set env var values made this nonsensical stuff
break.

Now we just have to do the obvious thing: set the env var to a bad
value, assert that (setting/get-value-of-type :string :site-url) is that
value, but assert that the _setting_ is a well-formatted value or nil if
we can't solve it (the "asd_12w31%$;" case).

Setting the env value then setting a temporary value to nil didn't make
much sense. And the fix made that set the env var to nil.

* fixup last tests

* Don't modify things from `with-temp-env-var-value`

this allows us to set non-mb-prefixed env vars. not sure if necessary
but let's roll with it.

* fix last of the tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.