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

Included file path is based on including file directory #114

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,31 @@ This is a port of the TypesafeConfig library to C++.

The library provides C++ support for the [HOCON configuration file format](https://github.com/typesafehub/config/blob/master/HOCON.md).

## I Made Some Change
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably makes more sense as the commit message rather than part of the README.


Error occured when I was using cpp-hocon to parse some config files in our project. That motivates me to fork the origin project of cpp-hocon, and change some code to fulfill my goal in using it. I'm not sure what I've done is right, and I don't know where anyone else is in the same situation. Next I'll tell you what my problem is, and how I fix it even in not-sure-correct way.

## My Problem

I'll use a simple example case to illustrate what the problem is. As is shown in the following image, in my example directory exist two sub_dirs, "bin" and "conf".

![image](imgs/tree1.png)

Under conf directory lays two config file, "a.conf" and "b.conf". The content of a.conf is *{ Peter : { include file("b.conf") } }* , and the content of b.conf is *{ passwd1 : "asdf" }*. And the following image is test code.

![image](imgs/test_code.png)

When I compile the code in directory "bin", and run the binary in "bin", I got an error. ![image](imgs/error.png)

However when I run the binary in directory "conf", I could get the expected outputs.

## My Solution

I thought it was the parser that can not parse included file in a relative path based on the directory of including file. Then I change some code to make it happen.

I am not sure what I've done is the right way to solve this problem. And I also don't know what kinds of bugs this change would make. So I post my code (both my change code and test code) to you, and hoping someone have a better idea to fix it.


```
MMMMMMMMMMMMMMMMMMMM
.====================.
Expand Down
2 changes: 2 additions & 0 deletions example/bin/README
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
1. compile the code
2. run binary in "bin" or in "conf"
26 changes: 26 additions & 0 deletions example/bin/conf_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include <hocon/parser/config_document_factory.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved to the lib/test directory and integrated with our test suite.

#include <hocon/config.hpp>

#include <iostream>
#include <set>

using namespace hocon;
using namespace std;

int main() {
auto conf_file = config::parse_file_any_syntax("../conf/a.conf");
auto conf = conf_file->resolve();
auto conf2 = config::parse_file_any_syntax("../conf/sub/b.conf")->resolve();
try {
auto str = conf->get_string("Peter.passwd1");
cout << "peter's passwd:" << str << endl;
cout << "peter's passwd2:" << conf->get_string("Peter.passwd2") << endl;
cout << "peter's passwd3:" << conf->get_string("Peter.passwd3") << endl;
cout << "peter's passwd4:" << conf->get_string("Peter.passwd4") << endl;

cout << "nick_name:" << conf2->get_string("other_field.nick_name") << endl;
} catch (hocon::config_exception& e) {
cout << "ERROR:" << e.what() << endl;
}
return 0;
}
Empty file added example/conf/README
Empty file.
10 changes: 10 additions & 0 deletions example/conf/a.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of example/conf and imgs/ seem like they should be part of the test suite as well.

include file("d.conf")
Peter : {
include file("b.conf")
include file("sub/b.conf")
passwd4 : ${new_passwd}
}

}

4 changes: 4 additions & 0 deletions example/conf/b.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
passwd1 : "adsf"
include file("/export/source_code/cpp-hocon/test/example/conf/sub/c.conf")
}
3 changes: 3 additions & 0 deletions example/conf/d.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
new_passwd : "qwer.,m"
}
7 changes: 7 additions & 0 deletions example/conf/sub/b.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
passwd2 : "lsdk" ,

other_field : {
include file("e.conf")
}
}
3 changes: 3 additions & 0 deletions example/conf/sub/c.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
passwd3 : "123414",
}
3 changes: 3 additions & 0 deletions example/conf/sub/e.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
nick_name : nick
}
Binary file added imgs/a_content.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/b_content.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/error.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/ok.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/test_code.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/tree1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added imgs/tree2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion lib/inc/hocon/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <vector>
#include <string>
#include <set>
#include <memory>
#include "export.h"
#include <leatherman/locale/locale.hpp>

Expand Down Expand Up @@ -208,7 +209,7 @@ namespace hocon {
* parse options
* @return the parsed configuration
*/
static shared_config parse_file_any_syntax(std::string file_basename, config_parse_options options);
static shared_config parse_file_any_syntax(std::string file_basename, config_parse_options options, shared_full_current fpath);

/**
* Like {@link #parseFileAnySyntax(File,ConfigParseOptions)} but always uses
Expand Down
7 changes: 7 additions & 0 deletions lib/inc/hocon/config_include_context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ namespace hocon {
*/
class LIBCPP_HOCON_EXPORT config_include_context {
public:

config_include_context() : _fpath(nullptr) {};
config_include_context(shared_full_current fpath) : _fpath(fpath) {};

/**
* Tries to find a name relative to whatever is doing the including, for
* example in the same directory as the file doing the including. Returns
Expand All @@ -46,6 +50,9 @@ namespace hocon {
* @return the parse options
*/
virtual config_parse_options parse_options() const = 0;

public:
shared_full_current _fpath;
};

} // namespace hocon
3 changes: 3 additions & 0 deletions lib/inc/hocon/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,7 @@ namespace hocon {

class config_parseable;
using shared_parseable = std::shared_ptr<const config_parseable>;

class full_path_operator;
using shared_full_current = std::shared_ptr<full_path_operator>;
} // namespace hocon
30 changes: 30 additions & 0 deletions lib/inc/internal/config_util.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#pragma once

#include <string>
#include <stack>
#include <mutex>
#include <unordered_map>
#include <memory>

namespace hocon {

Expand All @@ -14,4 +18,30 @@ namespace hocon {

std::string render_string_unquoted_if_possible(std::string const& s);

void extract_filename_from_path(const std::string& path,
std::string *file_dir,
std::string *file_name);

class full_path_operator {
public:
full_path_operator():
_current_dir(""),
_str_stack() {}

full_path_operator(const std::string& s):
_current_dir(s), _str_stack() {}

void stash();

void append(const std::string& dir);

int remove(const std::string& dir);

std::string extend(const std::string& str);

private:
std::string _current_dir;
std::stack<std::string> _str_stack;
};

} // namespace hocon
5 changes: 3 additions & 2 deletions lib/inc/internal/parseable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ namespace hocon {

class parseable : public config_parseable, public std::enable_shared_from_this<parseable> {
public:
static std::shared_ptr<parseable> new_file(std::string input_file_path, config_parse_options options);
static std::shared_ptr<parseable> new_file(std::string input_file_path, config_parse_options options, shared_full_current fpath=nullptr);
static std::shared_ptr<parseable> new_string(std::string s, config_parse_options options);
static std::shared_ptr<parseable> new_not_found(std::string what_not_found, std::string message,
config_parse_options options);

static config_syntax syntax_from_extension(std::string name);

void post_construct(config_parse_options const& base_options);
void post_construct(config_parse_options const& base_options, shared_full_current fpath);

std::shared_ptr<config_document> parse_config_document();
shared_object parse(config_parse_options const& options) const override;
Expand Down Expand Up @@ -74,7 +75,7 @@ namespace hocon {

class parseable_file : public parseable {
public:
parseable_file(std::string input_file_path, config_parse_options options);
parseable_file(std::string input_file_path, config_parse_options options, shared_full_current fpath);
std::unique_ptr<std::istream> reader() const override;
shared_origin create_origin() const override;
config_syntax guess_syntax() const override;
Expand Down
3 changes: 3 additions & 0 deletions lib/inc/internal/simple_include_context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ namespace hocon {
public:
// Include context is part of a parseable, so it can always expect a valid parseable reference.
simple_include_context(parseable const& parseable);
simple_include_context(parseable const& parseable, shared_full_current fpath);

// Unused method
// shared_include_context with_parseable(weak_parseable new_parseable) const;

shared_parseable relative_to(std::string file_name) const override;
config_parse_options parse_options() const override;

public:
shared_full_current _fpath;
private:
parseable const& _parseable;
};
Expand Down
11 changes: 7 additions & 4 deletions lib/inc/internal/simple_includer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <hocon/config_includer_file.hpp>
#include <hocon/config_parse_options.hpp>
#include <internal/full_includer.hpp>
#include <internal/config_util.hpp>

namespace hocon {

Expand All @@ -27,10 +28,12 @@ namespace hocon {

static shared_object from_basename(std::shared_ptr<name_source> source,
std::string name,
config_parse_options options);
config_parse_options options,
shared_full_current fpath);

static std::shared_ptr<const full_includer> make_full(std::shared_ptr<const config_includer> includer);

//static path_container _file_path_container;
private:
shared_includer _fallback;

Expand All @@ -57,23 +60,23 @@ namespace hocon {
class name_source {
public:
virtual shared_parseable name_to_parseable(std::string name,
config_parse_options parse_options) const = 0;
config_parse_options parse_options, shared_full_current fpath = nullptr) const = 0;
};

class relative_name_source : public name_source {
public:
relative_name_source(shared_include_context context);

shared_parseable name_to_parseable(std::string name,
config_parse_options parse_options) const override;
config_parse_options parse_options, shared_full_current fpath = nullptr) const override;
private:
const shared_include_context _context;
};

class file_name_source : public name_source {
public:
shared_parseable name_to_parseable(std::string name,
config_parse_options parse_options) const override;
config_parse_options parse_options, shared_full_current fpath = nullptr) const override;
};

} // namespace hocon
21 changes: 18 additions & 3 deletions lib/src/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <hocon/config_exception.hpp>
#include <internal/default_transformer.hpp>
#include <internal/resolve_context.hpp>
#include <internal/config_util.hpp>
#include <internal/values/config_boolean.hpp>
#include <internal/values/config_null.hpp>
#include <internal/values/config_number.hpp>
Expand All @@ -29,13 +30,27 @@ using namespace std;

namespace hocon {

shared_config config::parse_file_any_syntax(std::string file_basename, config_parse_options options) {
shared_config config::parse_file_any_syntax(std::string file_basename, config_parse_options options, shared_full_current fpath) {
std::string file_dir(""), file_name("");
if (nullptr == fpath) {
fpath = make_shared<full_path_operator>();
}
extract_filename_from_path(file_basename, &file_dir, &file_name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My instinct is that this isn't the place we should be fixing this, but it'd take some work for me to follow up on.

From a quick glance at https://github.com/lightbend/config/blob/7b9e1539dd46385e9b83be43c4112a448129ac2f/config/src/main/java/com/typesafe/config/impl/Parseable.java#L639-L655, I think the mistake may be in not overriding relative_to on parseable_file (and possibly others) in https://github.com/puppetlabs/cpp-hocon/blob/master/lib/src/parseable.cc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some pretty high-level comments. Do you have an interest in trying to improve this, or should we consider this an open problem for someone to build on your solution?

Thanks for code-reviewing and comments. I think we could left the problem open before solved. And I'll keep trying to figure another way out to fix it. Thanks again~
It's 3 in the morning and I need some sleep. Good day!

if (file_name.empty()) {
throw config_exception(_("can not find file '{1}' in Dir '{2}'", file_name, file_dir));
}

auto source = make_shared<file_name_source>();
return simple_includer::from_basename(move(source), move(file_basename), move(options))->to_config();
fpath->append(file_dir);
auto ret = simple_includer::from_basename(move(source), fpath->extend(file_name), move(options), fpath)->to_config();
if (fpath->remove(file_dir)) {
throw config_exception(_("removing file dir error"));
}
return ret;
}

shared_config config::parse_file_any_syntax(std::string file_basename) {
return parse_file_any_syntax(move(file_basename), config_parse_options::defaults());
return parse_file_any_syntax(move(file_basename), config_parse_options::defaults(), nullptr);
}

shared_config config::parse_string(string s, config_parse_options options)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/config_document_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ using namespace std;
namespace hocon { namespace config_document_factory {

shared_ptr<config_document> parse_file(string input_file_path, config_parse_options options) {
return parseable::new_file(move(input_file_path), move(options))->parse_config_document();
return parseable::new_file(move(input_file_path), move(options), nullptr)->parse_config_document();
}

shared_ptr<config_document> parse_file(string input_file_path) {
Expand Down
1 change: 1 addition & 0 deletions lib/src/config_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <internal/values/simple_config_object.hpp>
#include <internal/values/simple_config_list.hpp>
#include <leatherman/locale/locale.hpp>
#include <iostream>

// Mark string for translation (alias for leatherman::locale::format)
using leatherman::locale::_;
Expand Down
47 changes: 47 additions & 0 deletions lib/src/config_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,51 @@ namespace hocon {
return s;
}

void extract_filename_from_path(const std::string& path,
std::string *file_dir,
std::string *file_name) {
char sep = '/';
size_t i = path.rfind(sep, path.length());
if (string::npos != i) {
file_dir->assign(path.substr(0, i + 1));
file_name->assign(path.substr(i + 1, path.length() - i));
} else {
file_dir->assign("");
file_name->assign(path);
}
}

void full_path_operator::stash() {
_str_stack.push(_current_dir);
_current_dir.assign("");
}

void full_path_operator::append(const std::string& dir) {
if (! _current_dir.empty() && ! dir.empty() && dir[0] == '/') {
stash();
}
_current_dir.append(dir);
}

int full_path_operator::remove(const std::string& dir) {
size_t cur_dir_len = _current_dir.length();
size_t dir_len = dir.length();
if (cur_dir_len < dir_len) {
return -1;
}
if (_current_dir.substr(cur_dir_len - dir_len, dir_len).compare(dir)) {
return -2;
}
_current_dir.assign(_current_dir.substr(0, cur_dir_len - dir_len));
if (_current_dir.empty() && ! _str_stack.empty()) {
_current_dir.assign(_str_stack.top());
_str_stack.pop();
}
return 0;
}

std::string full_path_operator::extend(const std::string& str) {
return _current_dir + str;
}

} // namespace hocon
Loading