Skip to content

Commit 21f74d6

Browse files
drgrice1drgrice1
committed
Get rid of ENV hacking and fix the baseURL setting.
Add `getAssetURL` as a helper in the `RenderApp::Plugin::Assets` plugin module. This method should be used for any javascript or css files to ensure that the baseURL setting is correctly utilized. The only environment variables that are set are those that are needed for rendering. The renderer code itself should generally pass the controller object in where needed and configuration values should be extracted from there instead of setting environment variables corresponding to each configuration value. Also move the public/js/apps directories directly in public/js. This was done for webwork2 and PG a while ago, and should be done here as well.
1 parent 2d5ac98 commit 21f74d6

26 files changed

+314
-346
lines changed

‎lib/Renderer.pm‎

Lines changed: 78 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,35 @@
11
package Renderer;
22
use Mojo::Base 'Mojolicious';
33

4-
BEGIN {
5-
use Mojo::File;
6-
$main::libname = Mojo::File::curfile->dirname;
7-
8-
# RENDER_ROOT is required for initializing conf files.
9-
$ENV{RENDER_ROOT} = $main::libname->dirname
10-
unless (defined($ENV{RENDER_ROOT}));
11-
12-
# PG_ROOT is required for PG/lib/PGEnvironment.pm
13-
$ENV{PG_ROOT} = $main::libname . '/PG';
4+
use Mojo::File;
5+
use Env qw(RENDER_ROOT PG_ROOT baseURL);
6+
use Date::Format;
147

15-
# Used for reconstructing library paths from sym-links.
16-
$ENV{OPL_DIRECTORY} = "$ENV{RENDER_ROOT}/webwork-open-problem-library";
17-
18-
$ENV{MOJO_CONFIG} =
19-
(-r "$ENV{RENDER_ROOT}/renderer.conf")
20-
? "$ENV{RENDER_ROOT}/renderer.conf"
21-
: "$ENV{RENDER_ROOT}/renderer.conf.dist";
22-
$ENV{MOJO_LOG_LEVEL} = 'debug';
8+
BEGIN {
9+
# RENDER_ROOT and PG_ROOT are required for the WeBWorK::PG::Environment.
10+
$RENDER_ROOT = Mojo::File::curfile->dirname->dirname;
11+
$PG_ROOT = Mojo::File::curfile->dirname->child('PG');
2312
}
2413

25-
use lib "$main::libname";
26-
print "using root directory: $ENV{RENDER_ROOT}\n";
27-
2814
use Renderer::Model::Problem;
2915
use Renderer::Controller::IO;
3016
use WeBWorK::FormatRenderedProblem;
3117

3218
sub startup {
3319
my $self = shift;
3420

35-
# Merge environment variables with config file
3621
$self->plugin('Config');
37-
$self->plugin('TagHelpers');
3822
$self->secrets($self->config('secrets'));
39-
for (qw(problemJWTsecret webworkJWTsecret baseURL formURL SITE_HOST STRICT_JWT)) {
40-
$ENV{$_} //= $self->config($_);
41-
}
4223

43-
sanitizeHostURLs();
24+
$self->sanitizeHostURLs;
25+
26+
# This is also required for the WeBWorK::PG::Environment, but is not needed at compile time.
27+
$baseURL = $self->config->{baseURL};
4428

45-
print "Renderer is based at $main::basehref\n";
46-
print "Problem attempts will be sent to $main::formURL\n";
29+
say 'Renderer is based at ' . $self->defaults->{baseHREF};
30+
say 'Problem attempts will be sent to ' . $self->defaults->{formURL};
31+
32+
$self->plugin('Renderer::Plugin::Assets');
4733

4834
# Handle optional CORS settings
4935
if (my $CORS_ORIGIN = $self->config('CORS_ORIGIN')) {
@@ -63,42 +49,62 @@ sub startup {
6349

6450
# Logging
6551
if ($ENV{MOJO_MODE} && $ENV{MOJO_MODE} eq 'production') {
66-
my $logPath = "$ENV{RENDER_ROOT}/logs/error.log";
67-
print "[LOGS] Running in production mode, logging to $logPath\n";
52+
my $logPath = $self->home->child('logs', 'error.log');
53+
say "[LOGS] Running in production mode, logging to $logPath";
6854
$self->log(Mojo::Log->new(
6955
path => $logPath,
7056
level => ($ENV{MOJO_LOG_LEVEL} || 'warn')
7157
));
7258
}
7359

7460
if ($self->config('INTERACTION_LOG')) {
75-
my $interactionLogPath = "$ENV{RENDER_ROOT}/logs/interactions.log";
76-
print "[LOGS] Saving interactions to $interactionLogPath\n";
61+
my $interactionLogPath = $self->home->child('logs', 'interactions.log');
62+
say "[LOGS] Saving interactions to $interactionLogPath";
7763
my $resultsLog = Mojo::Log->new(path => $interactionLogPath, level => 'info');
7864
$resultsLog->format(sub {
7965
my ($time, $level, @lines) = @_;
8066
my $start = shift(@lines);
81-
my $msg = join ", ", @lines;
82-
return sprintf "%s, %s, %s\n", $start, $time - $start, $msg;
67+
return sprintf "%s, %s, %s\n", $start, $time - $start, join(', ', @lines);
8368
});
8469
$self->helper(logAttempt => sub { shift; $resultsLog->info(@_); });
8570
}
8671

72+
my $resourceUsageLog = Mojo::Log->new(path => $self->home->child('logs', 'resource_usage.log'));
73+
$resourceUsageLog->format(sub {
74+
my ($time, $level, @lines) = @_;
75+
return '[' . time2str('%a %b %d %H:%M:%S %Y', time) . '] ' . join(', ', @lines) . "\n";
76+
});
77+
$self->helper(resourceUsageLog => sub { shift; return $resourceUsageLog->info(@_); });
78+
8779
# Models
88-
$self->helper(newProblem => sub { shift; Renderer::Model::Problem->new(@_) });
80+
$self->helper(newProblem => sub { my ($c, $args) = @_; Renderer::Model::Problem->new($c, $args) });
8981

9082
# Helpers
91-
$self->helper(format => sub { WeBWorK::FormatRenderedProblem::formatRenderedProblem(@_) });
92-
$self->helper(validateRequest => sub { Renderer::Controller::IO::validate(@_) });
93-
$self->helper(parseRequest => sub { Renderer::Controller::Render::parseRequest(@_) });
94-
$self->helper(croak => sub { Renderer::Controller::Render::croak(@_) });
95-
$self->helper(logID => sub { shift->req->request_id });
96-
$self->helper(exception => sub { Renderer(@_) });
83+
$self->helper(
84+
format => sub {
85+
my ($c, $rh_result) = @_;
86+
WeBWorK::FormatRenderedProblem::formatRenderedProblem($c, $rh_result);
87+
}
88+
);
89+
$self->helper(validateRequest => sub { my ($c, $options) = @_; Renderer::Controller::IO::validate($c, $options) });
90+
$self->helper(parseRequest => sub { my $c = shift; Renderer::Controller::Render::parseRequest($c) });
91+
$self->helper(
92+
croak => sub {
93+
my ($c, $exception, $depth) = @_;
94+
Renderer::Controller::Render::croak($c, $exception, $depth);
95+
}
96+
);
97+
$self->helper(logID => sub { my $c = shift; $c->req->request_id });
98+
$self->helper(
99+
exception => sub {
100+
my ($c, $message, $status, %data) = @_;
101+
Renderer::Controller::Render::exception($c, $message, $status, %data);
102+
}
103+
);
97104

98105
# Routes
99-
# baseURL sets the root at which the renderer is listening,
100-
# and is used in Environment for pg_root_url
101-
my $r = $self->routes->under($ENV{baseURL});
106+
# baseURL is the root at which the renderer is listening.
107+
my $r = $self->routes->under($self->config->{baseURL});
102108

103109
$r->any('/render-api')->to('render#problem');
104110
$r->any('/render-ptx')->to('render#render_ptx');
@@ -108,10 +114,12 @@ sub startup {
108114
supplementalRoutes($r) if ($self->mode eq 'development' || $self->config('FULL_APP_INSECURE'));
109115

110116
# Static file routes
111-
$r->any('/pg_files/CAPA_Graphics/*static')->to('StaticFiles#CAPA_graphics_file');
112-
$r->any('/pg_files/tmp/*static')->to('StaticFiles#temp_file');
113-
$r->any('/pg_files/*static')->to('StaticFiles#pg_file');
114-
$r->any('/*static')->to('StaticFiles#public_file');
117+
$r->any('/pg_files/CAPA_Graphics/*static')->to('StaticFiles#CAPA_graphics_file')->name('capaFile');
118+
$r->any('/pg_files/tmp/*static')->to('StaticFiles#temp_file')->name('pgTempFile');
119+
$r->any('/pg_files/*static')->to('StaticFiles#pg_file')->name('pgFile');
120+
$r->any('/*static')->to('StaticFiles#public_file')->name('publicFile');
121+
122+
return;
115123
}
116124

117125
sub supplementalRoutes {
@@ -156,45 +164,40 @@ sub timeout {
156164
}
157165

158166
sub sanitizeHostURLs {
159-
$ENV{SITE_HOST} =~ s!/$!!;
160-
161-
# set an absolute base href for asset urls under iframe embedding
162-
if ($ENV{baseURL} =~ m!^https?://!) {
167+
my $self = shift;
163168

164-
# this should only be used by MITM sites when proxying renderer assets
165-
my $baseURL = $ENV{baseURL} =~ m!/$! ? $ENV{baseURL} : "$ENV{baseURL}/";
166-
$main::basehref = Mojo::URL->new($baseURL);
169+
$self->config->{SITE_HOST} =~ s!/$!!;
167170

168-
# do NOT use the proxy address in our router!
169-
$ENV{baseURL} = '';
170-
} elsif ($ENV{baseURL} =~ m!\S!) {
171+
# Set an absolute base href for asset urls under iframe embedding.
172+
if ($self->config->{baseURL} =~ m!^https?://!) {
173+
# This should only be used by MITM sites when proxying renderer assets.
174+
my $baseURL = $self->config->{baseURL} =~ m!/$! ? $self->config->{baseURL} : $self->config->{baseURL} . '/';
175+
$self->defaults->{baseHREF} = Mojo::URL->new($baseURL);
171176

172-
# ENV{baseURL} is used to build routes, so configure as "/extension"
173-
$ENV{baseURL} = "/$ENV{baseURL}";
174-
warn "*** [CONFIG] baseURL should not end in a slash\n"
175-
if $ENV{baseURL} =~ s!/$!!;
176-
warn "*** [CONFIG] baseURL should begin with a slash\n"
177-
unless $ENV{baseURL} =~ s!^//!/!;
177+
# Do NOT use the proxy address for the router!
178+
$self->config->{baseURL} = '';
179+
} elsif ($self->config->{baseURL} =~ m!\S!) {
180+
# Ensure baseURL starts with a slash but doesn't end with a slash.
181+
$self->config->{baseURL} = '/' . $self->config->{baseURL} unless $self->config->{baseURL} =~ m!^/!;
182+
$self->config->{baseURL} =~ s!/$!!;
178183

179-
# base href must end in a slash when not hosting at the root
180-
$main::basehref =
181-
Mojo::URL->new($ENV{SITE_HOST})->path("$ENV{baseURL}/");
184+
# base href must end in a slash when not hosting at the root.
185+
$self->defaults->{baseHREF} = Mojo::URL->new($self->config->{SITE_HOST})->path($self->config->{baseURL} . '/');
182186
} else {
183187
# no proxy and service is hosted at the root of SITE_HOST
184-
$main::basehref = Mojo::URL->new($ENV{SITE_HOST});
188+
$self->defaults->{baseHREF} = Mojo::URL->new($self->config->{SITE_HOST});
185189
}
186190

187-
if ($ENV{formURL} =~ m!\S!) {
188-
191+
if ($self->config->{formURL} =~ m!\S!) {
189192
# this should only be used by MITM
190-
$main::formURL = Mojo::URL->new($ENV{formURL});
193+
$self->defaults->{formURL} = Mojo::URL->new($self->config->{formURL});
191194
die '*** [CONFIG] if provided, formURL must be absolute'
192-
unless $main::formURL->is_abs;
195+
unless $self->defaults->{formURL}->is_abs;
193196
} else {
194197
# if using MITM proxy base href + renderer api not at SITE_HOST root
195198
# provide form url as absolute SITE_HOST/extension/render-api
196-
$main::formURL =
197-
Mojo::URL->new($ENV{SITE_HOST})->path("$ENV{baseURL}/render-api");
199+
$self->defaults->{formURL} =
200+
Mojo::URL->new($self->config->{SITE_HOST})->path($self->config->{baseURL} . '/render-api');
198201
}
199202
}
200203

‎lib/Renderer/Controller/IO.pm‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package Renderer::Controller::IO;
2-
use Mojo::Base -async_await;
3-
use Mojo::Base 'Mojolicious::Controller';
2+
use Mojo::Base 'Mojolicious::Controller', -async_await;
3+
44
use File::Spec::Functions qw(splitdir);
55
use File::Find qw(find);
66
use MIME::Base64 qw(decode_base64);
@@ -37,7 +37,7 @@ sub raw {
3737
return unless $validatedInput;
3838

3939
my $file_path = $validatedInput->{sourceFilePath};
40-
my $problem = $c->newProblem({ log => $c->log, read_path => $file_path });
40+
my $problem = $c->newProblem({ read_path => $file_path });
4141
$problem->{action} = 'fetch source';
4242
return $c->exception($problem->{_message}, $problem->{status})
4343
unless $problem->success();
@@ -58,7 +58,6 @@ async sub writer {
5858
return unless $validatedInput;
5959

6060
my $problem = $c->newProblem({
61-
log => $c->log,
6261
write_path => $validatedInput->{writeFilePath},
6362
problem_contents => $validatedInput->{problemSource}
6463
});
@@ -362,7 +361,7 @@ async sub findNewVersion {
362361
my $avoidProblems = {};
363362
$c->render_later;
364363
for my $seed (@avoidSeeds) {
365-
my $problem = $c->newProblem({ log => $c->log, read_path => $filePath, random_seed => $seed });
364+
my $problem = $c->newProblem({ read_path => $filePath, random_seed => $seed });
366365
my $renderedProblem = await $problem->render({});
367366
next unless ($problem->success());
368367
$avoidProblems->{$seed} = decode_json($renderedProblem);
@@ -375,7 +374,7 @@ async sub findNewVersion {
375374
$newSeed = 1 + int rand(999999);
376375
} until (!exists($avoidProblems->{$newSeed}));
377376

378-
my $newProblemObj = $c->newProblem({ log => $c->log, read_path => $filePath, random_seed => $newSeed });
377+
my $newProblemObj = $c->newProblem({ read_path => $filePath, random_seed => $newSeed });
379378
my $newProblemJson = await $newProblemObj->render({});
380379
next unless ($newProblemObj->success());
381380
$newProblem = decode_json($newProblemJson);
@@ -453,7 +452,7 @@ async sub findUniqueSeeds {
453452
do {
454453
$newSeed = 1 + int rand(999999);
455454
} until (!exists($triedSeeds->{$newSeed}));
456-
my $newProblemObj = $c->newProblem({ log => $c->log, read_path => $filePath, random_seed => $newSeed });
455+
my $newProblemObj = $c->newProblem({ read_path => $filePath, random_seed => $newSeed });
457456
my $newProblemJson = await $newProblemObj->render({});
458457
next unless ($newProblemObj->success());
459458
$newProblem = decode_json($newProblemJson);
@@ -519,7 +518,7 @@ async sub setTags {
519518
# the same holds for keywords
520519
$incomingTags->{keywords} = [ $incomingTags->{keywords} ] unless (ref($incomingTags->{keywords}) =~ /ARRAY/);
521520

522-
my $problem = $c->newProblem({ log => $c->log, read_path => $incomingTags->{file} });
521+
my $problem = $c->newProblem({ read_path => $incomingTags->{file} });
523522

524523
# wrap the get/update/write tags in a promise
525524
my $tags = WeBWorK::Utils::Tags->new($incomingTags->{file});

‎lib/Renderer/Controller/Render.pm‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ sub parseRequest {
1515
// '' =~ s!^\s*(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}).*$!$1!r;
1616
$originIP ||= $c->tx->remote_address || 'unknown-origin';
1717

18-
if ($ENV{STRICT_JWT} && !(defined $params{problemJWT} || defined $params{sessionJWT})) {
19-
return $c->exception('Not allowed to request problems with raw data.', 403);
18+
if ($c->config->{STRICT_JWT} && !(defined $params{problemJWT} || defined $params{sessionJWT})) {
19+
$c->exception('Not allowed to request problems with raw data.', 403);
20+
return;
2021
}
2122

2223
# protect against DOM manipulation
@@ -41,8 +42,8 @@ sub parseRequest {
4142
eval {
4243
$claims = decode_jwt(
4344
token => $sessionJWT,
44-
key => $ENV{webworkJWTsecret},
45-
verify_iss => $ENV{SITE_HOST},
45+
key => $c->config->{webworkJWTsecret},
46+
verify_iss => $c->config->{SITE_HOST},
4647
);
4748
1;
4849
} or do {
@@ -65,8 +66,8 @@ sub parseRequest {
6566
eval {
6667
$claims = decode_jwt(
6768
token => $problemJWT,
68-
key => $ENV{problemJWTsecret},
69-
verify_aud => $ENV{SITE_HOST},
69+
key => $c->config->{problemJWTsecret},
70+
verify_aud => $c->config->{SITE_HOST},
7071
);
7172
1;
7273
} or do {
@@ -78,12 +79,12 @@ sub parseRequest {
7879
@params{ keys %$claims } = values %$claims;
7980
} elsif ($params{outputFormat} ne 'ptx') {
8081
# if no JWT is provided, create one (unless this is a pretext request)
81-
$params{aud} = $ENV{SITE_HOST};
82+
$params{aud} = $c->config->{SITE_HOST};
8283
$params{isInstructor} //= 0;
8384
$params{sessionID} ||= time;
8485
my $req_jwt = encode_jwt(
8586
payload => \%params,
86-
key => $ENV{problemJWTsecret},
87+
key => $c->config->{problemJWTsecret},
8788
alg => 'PBES2-HS512+A256KW',
8889
enc => 'A256GCM',
8990
auto_iat => 1
@@ -156,7 +157,6 @@ async sub problem {
156157
}
157158

158159
my $problem = $c->newProblem({
159-
log => $c->log,
160160
read_path => $file_path,
161161
random_seed => $random_seed,
162162
problem_contents => $problem_contents
@@ -232,7 +232,7 @@ async sub sendAnswerJWT {
232232
message => 'initial message'
233233
};
234234
my $header = {
235-
Origin => $ENV{SITE_HOST},
235+
Origin => $c->config->{SITE_HOST},
236236
'Content-Type' => 'text/plain',
237237
};
238238

@@ -304,10 +304,10 @@ sub jweFromRequest {
304304
my $c = shift;
305305
my $inputs_ref = $c->parseRequest;
306306
return unless $inputs_ref;
307-
$inputs_ref->{aud} = $ENV{SITE_HOST};
307+
$inputs_ref->{aud} = $c->config->{SITE_HOST};
308308
my $req_jwt = encode_jwt(
309309
payload => $inputs_ref,
310-
key => $ENV{problemJWTsecret},
310+
key => $c->config->{problemJWTsecret},
311311
alg => 'PBES2-HS512+A256KW',
312312
enc => 'A256GCM',
313313
auto_iat => 1
@@ -319,10 +319,10 @@ sub jwtFromRequest {
319319
my $c = shift;
320320
my $inputs_ref = $c->parseRequest;
321321
return unless $inputs_ref;
322-
$inputs_ref->{aud} = $ENV{SITE_HOST};
322+
$inputs_ref->{aud} = $c->config->{SITE_HOST};
323323
my $req_jwt = encode_jwt(
324324
payload => $inputs_ref,
325-
key => $ENV{problemJWTsecret},
325+
key => $c->config->{problemJWTsecret},
326326
alg => 'HS256',
327327
auto_iat => 1
328328
);

0 commit comments

Comments
 (0)