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

Escaping Backslashes when interpolating code content #5112

Merged

Conversation

jimilp7
Copy link
Contributor

@jimilp7 jimilp7 commented May 26, 2024

Description

Problem:
Runtime crashes occurred due to improper escaping of backslashes in raw string patterns used in code blocks, exemplified by:

foo = re.match(r'[A-Z]+(\d+)', segments[2])

This resulted in errors such as:

server-1  | KeyError: '\\d'

The root cause was traced back to insufficient backslash escaping in the escaped_code variable during dynamic string operations.

Key Changes:

  • Enhanced Backslash Escaping: Updated the escaping mechanism to ensure correct handling of backslashes in dynamically evaluated strings. This change doubles backslashes in escaped_code, preventing runtime errors during regex operations. The adjustment specifically affects the __interpolate_code_content method, where re.sub() is utilized for string replacements.

How to Test:

  • Reproduce the Error: Before applying the changes, insert a raw string with an unescaped backslash in any Mage code block and execute it to observe the crash.
  • Verify the Fix: After implementing the changes, rerun the same block. The absence of the previous KeyError confirms the correction.

Checklist

  • The PR is tagged with proper labels (bug, enhancement, feature, documentation)
  • I have performed a self-review of my own code
  • I have added unit tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

cc:
@wangxiaoyou1993
@dy46
@tommydangerous

###PR Description
Problem:
Runtime crashes occurred due to improper escaping of backslashes in raw string patterns used in code blocks, exemplified by:

```
foo = re.match(r'[A-Z]+(\d+)', segments[2])
```

This resulted in errors such as:

```
server-1  | KeyError: '\\d'
```

The root cause was traced back to insufficient backslash escaping in the escaped_code variable during dynamic string operations.

###Key Changes:
- Enhanced Backslash Escaping: Updated the escaping mechanism to ensure correct handling of backslashes in dynamically evaluated strings. This change doubles backslashes in escaped_code, preventing runtime errors during regex operations. The adjustment specifically affects the __interpolate_code_content method, where re.sub() is utilized for string replacements.

###How to Test:
- Reproduce the Error: Before applying the changes, insert a raw string with an unescaped backslash in any Mage code block and execute it to observe the crash.
- Verify the Fix: After implementing the changes, rerun the same block. The absence of the previous KeyError confirms the correction.
@wangxiaoyou1993 wangxiaoyou1993 merged commit db89a01 into mage-ai:master May 28, 2024
5 checks passed
@wangxiaoyou1993 wangxiaoyou1993 mentioned this pull request May 29, 2024
2 tasks
wangxiaoyou1993 added a commit that referenced this pull request May 30, 2024
# Description
<!-- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->
 Bump version to 0.9.71
<!-- Release notes generated using configuration in .github/release.yml
at release/0.9.71 -->

## What's Changed
### 🎉 Exciting New Features
* [xh]Create streaming oracle destination by @matrixstone in
#4896
### 🐛 Bug Fixes
* [xy] Exclude dropped columns from the postgres schema discovery query.
by @wangxiaoyou1993 in #5002
* [xy] Fix None Bigquery database error. by @wangxiaoyou1993 in
#5005
* [xy] Fix extracting table name from INSERT statement in SQL block. by
@wangxiaoyou1993 in #5008
* [xy] Catch yaml interpolation error. by @wangxiaoyou1993 in
#5012
* [xy] Not run insert query for empty dataframe. by @wangxiaoyou1993 in
#5017
* [dy] Fix git and seed issues by @dy46 in
#5025
* [xy] Fix custom template issue on server start. by @wangxiaoyou1993 in
#5038
* [xy] Comment out flaky unit tests by @wangxiaoyou1993 in
#5046
* [jk] Allow files to be selected for pipeline zip import by
@johnson-mage in #5042
* [dy] Update git email field by @dy46 in
#5045
* [dy] Fix circular dependency by @dy46 in
#5050
* [td] Update charts and make a ton of bug fixes by @tommydangerous in
#5075
* [td] Fix IDE outputs by @tommydangerous in
#5088
* [td] Fix git issue by @tommydangerous in
#5089
* Td fix git2 by @tommydangerous in
#5090
* [td] Upgrade deltalake because of Docker image build error by
@tommydangerous in #5091
* [xy] Fix websocket authentication. by @wangxiaoyou1993 in
#5094
* [td] Fix interpolating dictionaries when adding custom code to ide
execution by @tommydangerous in
#5095
* [xy] Fix switching active kernel by @wangxiaoyou1993 in
#5098
* [xy] Fix retry in streaming pipeline. by @wangxiaoyou1993 in
#5099
* [xy] Fix performance issue of fetching variables. by @wangxiaoyou1993
in #5108
* [jk] Fix max concurrent runs value for backfill by @johnson-mage in
#5115
* [xy] Fix circular dependency. by @wangxiaoyou1993 in
#5122
* [td] Don’t raise error if can’t load by @tommydangerous in
#5132
* [xy] Fix custom template creation. by @wangxiaoyou1993 in
#5134
* [xy] Fix variables API and BaseEnum by @wangxiaoyou1993 in
#5135
* [td] Fix after on pipeline dashboard by @tommydangerous in
#5142
* [td] Fix after panel showing up everywhere by @tommydangerous in
#5143
### 💅 Enhancements & Polish
* [xy] Support configuring logs_dir_path in environment variable. by
@wangxiaoyou1993 in #5018
* [dy] Add prune option for fetch by @dy46 in
#5014
* [xy] Include server and scheduler process id in status response. by
@wangxiaoyou1993 in #5027
* [xy] Add pipeline_run_id to kwargs. by @wangxiaoyou1993 in
#5030
* [xy] Allow configuring authentication mode in mssql config. by
@wangxiaoyou1993 in #5051
* Add configuration parameters to V1JobSpec in k8s executor by @artche
in #5044
* Reload libraries when running blocks from edit pipeline by @hugabora
in #4953
* [xy] Support configuring concurrency config via env vars. by
@wangxiaoyou1993 in #5057
* [xy] Some improvements on multi project by @wangxiaoyou1993 in
#5063
* Add OIDC_ROLES_MAPPING support by @mihaivint in
#5053
* [jk] Improve visibility of variable/secret names by @johnson-mage in
#5072
* [xy] Cache project platform settings data in API calls. by
@wangxiaoyou1993 in #5079
* [td] Memory upgrades by @tommydangerous in
#5092
* [td] Track resource usage and run block execution function in a
context by @tommydangerous in
#5105
* [td] Dynamic blocks use the new memory enhancements by @tommydangerous
in #5106
* [xy] Speed up pipeline update endpoint. by @wangxiaoyou1993 in
#5125
* [td] Dynamic block stream mode by @tommydangerous in
#5121
* [xy] Disable autoreload in non-dev environments. by @wangxiaoyou1993
in #5136
* [xy] Reduce CPU usage in scheduler. by @wangxiaoyou1993 in
#5137
### Other Changes
* relax pytz version requirements by @alex-hunsaker in
#5006
* [xy] Not show locals in CLI exception. by @wangxiaoyou1993 in
#5009
* [dy] Remove error pop up by @dy46 in
#5015
* Add support for user_roles when using OIDC auth by @mihaivint in
#4899
* upgrade deltalake for tableMerger by @oonyoontong in
#5020
* [dy] Update multi project platform to support active project per user
by @dy46 in #4865
* docs: typo in architecture.mdx by @stefaan1o in
#5034
* Update setup-dbt.mdx by @richardlhughes in
#5033
* [tc] Update releases.mdx by @thomaschung408 in
#5056
* [td] Fix charts when using Spark by @tommydangerous in
#5028
* [td] Fix interactions, triggers, dynamic blocks, charts, and more by
@tommydangerous in #5064
* [td] Remove PYTHONPATH from docker-compose by @tommydangerous in
#5073
* [td] Turn off unoptimized images by @tommydangerous in
#5082
* Grammar corrections to doc "Update non-root-user.mdx" by @CLHdevOps in
#5077
* [td] Fix infer variables by @tommydangerous in
#5102
* [td] Dynamic block resilience by @tommydangerous in
#5113
* Escaping Backslashes when interpolating code content by @jimilp7 in
#5112
* Add pipeline_schedule_description variable to notification sender by
@hjhdaniel in #5118


# How Has This Been Tested?
<!-- Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
-->

- [x] Comment tests locally
- [x] Performance tests

# Checklist
N/A

cc:
<!-- Optionally mention someone to let them know about this pull request
-->
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.

None yet

2 participants